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

Fix many ruby 2.7 warnings for the 6.0 stable branch #37935

Merged
merged 5 commits into from Dec 16, 2019

Conversation

casperisfine
Copy link
Contributor

When possible I simply cherry-picked fixes from the master branch, but sometimes I had to redo them manually.

Here I started with ActiveSupport, I'm not sure I eliminated them all yet because the test suite is segfaulting on my machine.

This is still a work in progress, but opening a PR give me a CI run which is helpful.

cc @rafaelfranca @Edouard-chin

since RedisCacheStore#write_entry takes kwargs, we needed to kwargsify all these methods
in order to eliminate Ruby 2.7 warnings.

It's a little bit bigger patch than I expected, but it doesn't warn on Ruby 3,
and it doesn't introduce any incompatibility on loder rubies, so it may not be a bad thing anyway.
@kaspth
Copy link
Contributor

kaspth commented Dec 15, 2019

Looks like the build is green, do you want to get this part merged and start up a PR for the remaining warnings or just all in one?

@casperisfine casperisfine changed the title [WIP] Fix many ruby 2.7 warnings for the 6.0 stable branch Fix many ruby 2.7 warnings for the 6.0 stable branch Dec 16, 2019
@casperisfine
Copy link
Contributor Author

@kaspth there was only one actionable warning left in activesupport, so I update the PR.

I think it should be good to go after that.

There is still a huge chunk of warnings coming from tzinfo, but to get them fixed we need to upgrade to a new major version of tzinfo, so it doesn't seem ok for a tiny release.

@casperisfine
Copy link
Contributor Author

Oh and the suite breaks because of a Deadlock, but it's also the case on master.

@kaspth kaspth merged commit 9ac03a2 into rails:6-0-stable Dec 16, 2019
@kaspth
Copy link
Contributor

kaspth commented Dec 16, 2019

Thanks!

There is still a huge chunk of warnings coming from tzinfo, but to get them fixed we need to upgrade to a new major version of tzinfo, so it doesn't seem ok for a tiny release.

Yeah, that'll probably take some work. What do you mean about tiny release? Should be perfectly safe to require a newer tzinfo for 6.1, no?

@casperisfine
Copy link
Contributor Author

What do you mean about tiny release

Tiny as in MAJOR.MINOR.TINY. Only the new 2.0 has Ruby 2.7 fixes, but the interface changed a lot (see master...Shopify:update-tzinfo-2.0, and it's not even green yet).

So it would be complicated to bump tzinfo to 2.0 in Rails 6.0.X.

That's why I started working on a PR to tzinfo to get a new 2.7 compatible 1.x release: tzinfo/tzinfo#111

@kaspth
Copy link
Contributor

kaspth commented Dec 16, 2019

Perfect!

koic added a commit to koic/cancancan that referenced this pull request Jan 10, 2020
## Summary

This PR suppresses the following keyword arguments warning for Ruby 2.7.0.

```console
% ruby -v
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-darwin17]
% DB=sqlite bundle exec appraisal activerecord_6.0.0 rake

/Users/koic/src/github.com/CanCanCommunity/cancancan/lib/cancan/unauthorized_message_resolver.rb:9:
warning: Using the last argument as keyword parameters is deprecated;
maybe ** should be added to the call
/Users/koic/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/i18n-1.8.0/lib/i18n.rb:195:
warning: The called method `translate' is defined here
```

For Ruby 2.8.0-dev (Ruby 3.0) the warning will be `ArgumentError`.
ruby/ruby#2794

## Other Infromation

There are other similar warnings against Active Record, but Rails 6.0
stable branch is WIP.
rails/rails#37935

It is not subject to changes in this repository.
koic added a commit to koic/cancancan that referenced this pull request Jan 10, 2020
## Summary

This PR suppresses the following keyword arguments warning for Ruby 2.7.0.

```console
% ruby -v
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-darwin17]
% DB=sqlite bundle exec appraisal activerecord_6.0.0 rake

/Users/koic/src/github.com/CanCanCommunity/cancancan/lib/cancan/unauthorized_message_resolver.rb:9:
warning: Using the last argument as keyword parameters is deprecated;
maybe ** should be added to the call
/Users/koic/.rbenv/versions/2.7.0/lib/ruby/gems/2.7.0/gems/i18n-1.8.0/lib/i18n.rb:195:
warning: The called method `translate' is defined here
```

For Ruby 2.8.0-dev (Ruby 3.0) the warning will be `ArgumentError`.
ruby/ruby#2794

## Other Infromation

There are other similar warnings against Active Record, but Rails 6.0
stable branch is WIP.
rails/rails#37935

It is not subject to changes in this repository.
@lucascaton
Copy link
Contributor

Hi there! 👋

Is there any plans to release this fix in a v6.0.3 or something, to solve the warnings?

Thanks!

@casperisfine
Copy link
Contributor Author

I can't say when 6.0.3 will be released, AFAIK it still need a couple bugfixes. In the meantime you can point your Gemfile to the 6-0-stable branch.

@lucascaton
Copy link
Contributor

Got it, thanks @casperisfine! I thought it was fully fixed already :)

@sergey-alekseev
Copy link
Contributor

Rails 6.0.3 has been released 🎉


To @lucascaton and everyone else who is subscribed and waiting.

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

Successfully merging this pull request may close these issues.

None yet

6 participants