Skip to content

Update config.ru when running app:update task - #49704

Closed
bensheldon wants to merge 1 commit into
rails:mainfrom
bensheldon:update_configru
Closed

Update config.ru when running app:update task#49704
bensheldon wants to merge 1 commit into
rails:mainfrom
bensheldon:update_configru

Conversation

@bensheldon

@bensheldon bensheldon commented Oct 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation / Background

This PR ensures that the config.ru file is updated when running bin/rails app:update

Detail

This Pull Request has been created because changes were made to the default config.ru file to support a Railtie#server do hook in #39953 and merged via f8a1a8a).

Inspecting the initial PR vs the final commit, the PR contains the change to #config_when_updating, but the subsequent commit does not, so maybe just an oversight: https://github.com/rails/rails/pull/39953/files#diff-4521b0667f0d930737cfa0a1ced7dd5c00a15909503123cf3752a6c6ebfcca04R143

As a gem developer who would love to use this hook in my Railties/Engines, I've noticed that many older Rails apps have not updated their config.ru to trigger this hook. Which also means I've done gross workarounds, which I'd love the chance to slightly avoid.

Noted in bensheldon/good_job#1106 (comment)

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@rails-bot rails-bot Bot added the railties label Oct 19, 2023
@skipkayhil

skipkayhil commented Oct 19, 2023

Copy link
Copy Markdown
Member

Ref: #41083 where updating config.ru was removed

@bensheldon

bensheldon commented Oct 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@skipkayhil oh, that's a bummer, but I understand the intent, though I guess:

as they change very rarely at the framework level,

...may be true, but it's important that they do change when they do change.

I'll go back to the drawing board then. Maybe I could try logging a (Deprecation?) warning if say Rails::Server is defined but the server hook hasn't been called. Not sure how else to indicate to devs "hey, this framework file needs to be updated or you might be missing out on functionality"

@bensheldon bensheldon closed this Oct 19, 2023
@bensheldon

bensheldon commented Oct 19, 2023

Copy link
Copy Markdown
Contributor Author

Maybe I could try logging a (Deprecation?) warning if say Rails::Server is defined but the server hook hasn't been called

Oh, but that would need to live in middleware or like a before_action hook and that seems messy 🤔

@skipkayhil

skipkayhil commented Oct 19, 2023

Copy link
Copy Markdown
Member

Ah, sorry I didn't mean to say this wouldn't be accepted, just trying to add some context 😅

I agree it's unfortunate that there was only a single (6.1) version of Rails that updated config.ru and updated it with app:update. It seems like running app:update for each version is still not common knowledge for people. I think adding config.ru back to app:update could be a good change

@bensheldon bensheldon mentioned this pull request Jul 29, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

railties

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bensheldon @skipkayhil