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 a deprecation warning in unique_generator.rb related to the kwargs change in Ruby 2.7. #1868

Merged
merged 4 commits into from
Dec 28, 2019

Conversation

connorshea
Copy link
Member

This fixes a deprecation warning I was seeing when using Faker.unique:

/Users/connorshea/.rbenv/versions/2.7.0-preview3/lib/ruby/gems/2.7.0/gems/faker-2.9.0/lib/helpers/unique_generator.rb:22: warning: The last argument is used as the keyword parameter
/Users/connorshea/.rbenv/versions/2.7.0-preview3/lib/ruby/gems/2.7.0/gems/faker-2.9.0/lib/faker/default/number.rb:178: warning: for `between' defined here

It also removes ~6000 lines of deprecation warnings from the Faker test suite on Ruby 2.7. (There are still >250k others 😉)

See this post for more info: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

@connorshea
Copy link
Member Author

This is failing because it lacks the fixes from #1867, so I'll need to rebase it once that's merged.

@connorshea connorshea force-pushed the fix-deprecation-warning-2-7 branch from 2f1c38c to c491b8f Compare December 19, 2019 07:05
@connorshea
Copy link
Member Author

Actually, I think this passes the arguments to the deprecated positional arguments. Don't merge this yet.

This was only passing the test suite because of the
backwards compatible positional args that most Faker methods have. We
need to explicitly pass keyword args, separately from 'normal' args.
@connorshea
Copy link
Member Author

connorshea commented Dec 22, 2019

Hmm, I'm not sure what the correct way to fix this is :/ I'd rather avoid using ruby2_keywords, if possible.

@connorshea
Copy link
Member Author

connorshea commented Dec 27, 2019

It works now. But at what cost?

ruby2_keywords will eventually be removed in Ruby 3.2, so Christmas 2022 is going to bite us in the butt :P I'm somewhat skeptical they'll actually remove it in 3.2 rather than waiting until 3.3 or 3.4. We can replace it with ... once we drop support for any versions below Ruby 2.7, but until then it's not really an option :(

Copy link
Member

@vbrazo vbrazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@vbrazo vbrazo merged commit 5f04a40 into faker-ruby:master Dec 28, 2019
@connorshea connorshea deleted the fix-deprecation-warning-2-7 branch December 28, 2019 20:51
michebble pushed a commit to michebble/faker that referenced this pull request Feb 16, 2020
…s change in Ruby 2.7. (faker-ruby#1868)

* Fix a deprecation warning related to the kwargs change in Ruby 2.7.

* Try a different approach for fixing the deprecation.

* Actually fix deprecation warning.

This was only passing the test suite because of the
backwards compatible positional args that most Faker methods have. We
need to explicitly pass keyword args, separately from 'normal' args.

* I give up. ruby2_keywords it is.
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
…s change in Ruby 2.7. (faker-ruby#1868)

* Fix a deprecation warning related to the kwargs change in Ruby 2.7.

* Try a different approach for fixing the deprecation.

* Actually fix deprecation warning.

This was only passing the test suite because of the
backwards compatible positional args that most Faker methods have. We
need to explicitly pass keyword args, separately from 'normal' args.

* I give up. ruby2_keywords it is.
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.

2 participants