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

Ruby 2.6 suport for 5-2-stable branch #34720

Merged
merged 7 commits into from Dec 25, 2018

Conversation

Projects
None yet
5 participants
@yahonda
Copy link
Contributor

yahonda commented Dec 16, 2018

Summary

Ruby 2.6.0-rc2 Released and Ruby 2.6 will be released on this December 25. I expect Rails 5.2 should run with Ruby 2.6 without errors/warnings.

This pull request backports changes to 5-2-stable branch to suppress errors and warnings which have been already addressed in the master branch. I'm wondering if these kinds of backporting should be done by Rails committers and/or release manager. Let me open a pull request first.

Other Information

There are some errors remained. One can be "fixed" but it needs to backport pull requests with CHANGELOG. Another one, I do not know why it fails with 5-2-stable.

  • It has been addressed by #32938 to master branch. It has changelog entry then I did not backport it to this pull request.
$ cd activesupport
$ bundle exec ruby -w -Itest test/core_ext/range_ext_test.rb -n /test_should_compare/
Run options: -n /test_should_compare/ --seed 24989

# Running:

F

Failure:
RangeTest#test_should_compare_other_with_exclusive_end [test/core_ext/range_ext_test.rb:72]:
Expected false to be truthy.


bin/rails test test/core_ext/range_ext_test.rb:71

F

Failure:
RangeTest#test_should_compare_identical_exclusive [test/core_ext/range_ext_test.rb:68]:
Expected false to be truthy.


bin/rails test test/core_ext/range_ext_test.rb:67

F

Failure:
RangeTest#test_should_compare_identical_inclusive [test/core_ext/range_ext_test.rb:64]:
Expected false to be truthy.


bin/rails test test/core_ext/range_ext_test.rb:63



Finished in 0.001701s, 1763.5357 runs/s, 1763.5357 assertions/s.
3 runs, 3 assertions, 3 failures, 0 errors, 0 skips
  • test_restart_rails_server_with_custom_pid_file_path fails
    It passes with the master branch.
$ cd railties
$ bundle exec ruby -w -Itest test/application/server_test.rb -n test_restart_rails_server_with_custom_pid_file_path
Run options: -n test_restart_rails_server_with_custom_pid_file_path --seed 41272

# Running:

F

Failure:
ApplicationTests::ServerTest#test_restart_rails_server_with_custom_pid_file_path [test/application/server_test.rb:53]:
"Inherited" expected, but got:

...
Traceback (most recent call last):
	2: from bin/rails:4:in `<main>'
	1: from /home/yahonda/.rbenv/versions/2.6.0-rc2/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'
/home/yahonda/.rbenv/versions/2.6.0-rc2/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require': cannot load such file -- rails/commands (LoadError)
.
Expected "...\r\n\e[1mTraceback\e[m (most recent call last):\r\n\t2: from bin/rails:4:in `<main>'\r\n\t1: from /home/yahonda/.rbenv/versions/2.6.0-rc2/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require'\r\n/home/yahonda/.rbenv/versions/2.6.0-rc2/lib/ruby/2.6.0/rubygems/core_ext/kernel_require.rb:54:in `require': \e[1mcannot load such file -- rails/commands (\e[1;4mLoadError\e[m\e[1m)\e[m\r\n" to include "Inherited".


bin/rails test test/application/server_test.rb:35



Finished in 14.595938s, 0.0685 runs/s, 0.4111 assertions/s.
1 runs, 6 assertions, 1 failures, 0 errors, 0 skips
@yahonda

This comment has been minimized.

Copy link
Contributor

yahonda commented Dec 17, 2018

Looks like this pull request includes something not available for Ruby 2.2 and 2.3. Let me correct them.

@yahonda yahonda force-pushed the yahonda:5-2-stable_ruby26 branch 2 times, most recently from 314ff05 to a21ab24 Dec 17, 2018

@yahonda

This comment has been minimized.

Copy link
Contributor

yahonda commented Dec 17, 2018

Reverted e8237d1 since only Ruby 2.4 or higher supports Regexp#match?. Now the remaining failures at 5-2-stable are #32938 and #34719.

@ahorek

This comment has been minimized.

Copy link

ahorek commented Dec 17, 2018

Regexp#match? is supported https://github.com/rails/rails/blob/5-2-stable/activesupport/lib/active_support/core_ext/regexp.rb#L8 but String#match? isn't

I don't see a reason why #32695 should be backported. It's a performance optimization and warnings are from rubocop. It's not related to Ruby 2.6 compatibility at all.

Style/ParenthesesAroundCondition:
Enabled: true

Style/ParenthesesAroundCondition:

This comment has been minimized.

@kamipo

kamipo Dec 21, 2018

Member

Why is ff0af71 (and 48d1c48) necessary?

@yahonda yahonda force-pushed the yahonda:5-2-stable_ruby26 branch 3 times, most recently from eefd4f4 to 04ae2e4 Dec 24, 2018

rafaelfranca and others added some commits Apr 4, 2018

Merge pull request #32447 from utilum/splat
2.6 warnings: passing splat keyword arguments as a single Hash
Merge pull request #32556 from utilum/splat
2.6 warning: passing splat keyword arguments as a single Hash
Merge pull request #32612 from utilum/splat_actionview
Ruby 2.6 warning: passing splat keyword arguments as a single Hash
Restore original merging order to enforce `if_exists: true`
The merging order was accidentally changed at #32447. The original
intention is force `drop_table ... if_exists: true`. #28070.
Merge pull request #32799 from printercu/patch-6
Use usual method definition instead of extracting args from array
Merge pull request #32938 from utilum/range_case_equality
Allow Range#=== and Range#cover? on Range

@yahonda yahonda force-pushed the yahonda:5-2-stable_ruby26 branch from 04ae2e4 to bd24ef6 Dec 24, 2018

@yahonda yahonda changed the title [WIP] Ruby 2.6 suport for 5-2-stable branch Ruby 2.6 suport for 5-2-stable branch Dec 24, 2018

@yahonda

This comment has been minimized.

Copy link
Contributor

yahonda commented Dec 24, 2018

Thanks for the reviews.

I have dropped #32695, ff0af71 and 48d1c48 from this pull request changing .rubocop.yml
Also backported #32938 to 5-2-stable to support Ruby 2.6 new Range behavior.


Range.prepend(ActiveSupport::IncludeWithRange)
ActiveSupport::Deprecation.warn "You have required `active_support/core_ext/range/include_range`. " \

This comment has been minimized.

@kamipo

kamipo Dec 24, 2018

Member

The deprecation warning should not be happened in the next stable release.

This comment has been minimized.

@yahonda

yahonda Dec 24, 2018

Contributor

Removed this deprecation warning as far as I understand this file will not be removed in 5.2.x.

This comment has been minimized.

This comment has been minimized.

@yahonda

yahonda Dec 24, 2018

Contributor

Updated the changelog entry.

Remove deprecation warning about require `active_support/core_ext/ran…
…ge/include_range`

This file will not be removed in 5.2.

@yahonda yahonda force-pushed the yahonda:5-2-stable_ruby26 branch from 345ab0d to e12df15 Dec 24, 2018

@kamipo kamipo merged commit b4bb511 into rails:5-2-stable Dec 25, 2018

1 check passed

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

@yahonda yahonda deleted the yahonda:5-2-stable_ruby26 branch Jan 15, 2019

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