New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comment out config.file_watcher during Rails upgrade process #24243

Merged
merged 1 commit into from Mar 21, 2016

Conversation

Projects
None yet
5 participants
@dewski
Contributor

dewski commented Mar 18, 2016

This PR is a follow up to the feedback received in #24066 trying to fix the issue raised in #24063.

Rails 5 introduces a new ActiveSupport::EventedFileUpdateChecker class which is conditionally injected into existing Rails application configuration files from the bin/rails app:update task. During this upgrade process Rails checks to see if the listen library is required. If listen is required ActiveSupport::EventedFileUpdateChecker is added to the bottom of config/environments/development.rb as:

config.file_watcher = ActiveSupport::EventedFileUpdateChecker

Per @rafaelfranca's comments, the upgrade task should not require the installation of any new gems. Modifying the app generator to comment out the config.file_watcher line by default resolves the issue.

I've stored the full terminal output verifying the behavior here in a gist.

/cc @arthurnn @sadinie @maclover7 @rafaelfranca @fxn

@rails-bot

This comment has been minimized.

rails-bot commented Mar 18, 2016

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @arthurnn (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@arthurnn

This comment has been minimized.

Member

arthurnn commented Mar 21, 2016

merged 01ef3b7 , and tests added .

Thanks

@arthurnn arthurnn closed this Mar 21, 2016

@arthurnn arthurnn merged commit 57e258e into rails:master Mar 21, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

arthurnn added a commit that referenced this pull request Mar 21, 2016

arthurnn added a commit that referenced this pull request Mar 21, 2016

Merge pull request #24243 from dewski/dont-upgrade-file-watcher
Comment out config.file_watcher during Rails upgrade process
@fxn

This comment has been minimized.

Member

fxn commented Mar 21, 2016

Any reason not to just comment out the line as it is?

@fxn

This comment has been minimized.

Member

fxn commented Mar 21, 2016

Ah, forget it, can't read a patch :).

tute added a commit to thoughtbot/suspenders that referenced this pull request Apr 28, 2016

Upgrade to Rails 5.0.0.beta3
In preparation for Rails 5.

I needed to add the `listen` gem in development. See related PR to
Rails and its related issues: rails/rails#24243

Right now, generating an app shows:

```
conflict  config/puma.rb
Overwrite test-apr-28/config/puma.rb? (enter "h" for help) [Ynaqdh]
```

@tute tute referenced this pull request Apr 28, 2016

Merged

Upgrade to Rails 5.0.0 #753

tute added a commit to thoughtbot/suspenders that referenced this pull request May 3, 2016

Upgrade to Rails 5.0.0.beta
In preparation for Rails 5.

I needed to add the `listen` gem in development. See related PR to
Rails and its related issues: rails/rails#24243

tute added a commit to thoughtbot/suspenders that referenced this pull request May 3, 2016

Upgrade to Rails 5.0.0.beta
In preparation for Rails 5.

I needed to add the `listen` gem in development. See related PR to
Rails and its related issues: rails/rails#24243

y-yagi added a commit to y-yagi/rails that referenced this pull request Mar 22, 2017

Don't comment out config.file_watcher during Rails upgrade
This is necessary only when updating to Rails 5.0, it is not necessary
for updating to 5.1.

Related rails#24243
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment