Skip to content
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

The 'listen' gem should also be included on other Ruby implementations #35482

Merged
merged 1 commit into from
Mar 5, 2019

Conversation

eregon
Copy link
Contributor

@eregon eregon commented Mar 5, 2019

Summary

  • This if RUBY_ENGINE == 'ruby' was meant to only skip byebug since its introduction in 059bdc3. Listen was added in de6ad56.
  • Others gems already have appropriate checks for their inclusion.
  • Test that the listen gem is included when RUBY_ENGINE is not 'ruby'.

This is already fixed on master by #34243 (cc @deivid-rodriguez), but not yet for Rails 5.

Other Information

I noticed this while generating a Rails app on TruffleRuby. listen is not included in the Gemfile, leading to an error while loading Rails for bundle exec rails serve.

* This `if RUBY_ENGINE == 'ruby'` was meant to only skip `byebug` since
  its introduction in 059bdc3.
  Listen was added in de6ad56.
* Others gems already have appropriate checks for their inclusion.
* Test that the listen gem is included when RUBY_ENGINE is not 'ruby'.
@rails-bot rails-bot bot added the railties label Mar 5, 2019
@eregon
Copy link
Contributor Author

eregon commented Mar 5, 2019

@y-yagi Could you review this PR since you reviewed #34243 ? 😃

BTW, the buildkite failure seems unrelated to this PR.

@y-yagi y-yagi merged commit 2ee269f into rails:5-2-stable Mar 5, 2019
@y-yagi
Copy link
Member

y-yagi commented Mar 5, 2019

@eregon Thanks!

eregon added a commit to eregon/rails that referenced this pull request Apr 3, 2019
* The fix is already in master since rails#34243
* See rails#35482 for the fix in Rails 5.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants