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

Explicitly pass a parameter as hash to be more ruby 2.7 friendly #892

Merged
merged 4 commits into from
Jan 10, 2020

Conversation

dennisvdvliet
Copy link
Contributor

@dennisvdvliet dennisvdvliet commented Jan 9, 2020

Ruby 2.7 started emitting some warnings when you do things that are not ruby 3.0 compatible. The way Stripe::StripeObject.update_attributes raises a number of warnings.

For example in ruby 2.7 the following will raise a warning

obj = Stripe::StripeObject.construct_from(id: 1, name: "Stripe")
obj.update_attributes(name: 'Dennis')

To make this work in ruby 2.7 we need to explicitly pass in a hash as the first argument

obj = Stripe::StripeObject.construct_from(id: 1, name: "Stripe")
obj.update_attributes({name: 'Dennis'})

👮 👮‍♀ Rubocop does not really like this so that is why there are a number of comments added to the spec to allow this explicit hash passing.

[0] https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

@dennisvdvliet dennisvdvliet mentioned this pull request Jan 9, 2020
@brandur-stripe
Copy link
Contributor

@dennisvdvliet Thank you again! Two minor asks:

  1. Would you mind moving the Rubocop disable tags to each individual line instead of broadly disabling for the entire file? Just looks like this:

    obj.update_attributes({ name: "STRIPE" }) # rubocop:disable Style/BracesAroundHashParameters
  2. Can you take out the .ruby-version file included in the PR? Maybe we should have one of these in a project, but I'd prefer to bring it in on its own PR so that we consider it in isolation.

Thanks!

@dennisvdvliet dennisvdvliet marked this pull request as ready for review January 9, 2020 19:13
@dennisvdvliet
Copy link
Contributor Author

@brandur-stripe Done

@brandur-stripe
Copy link
Contributor

Excellent! Thanks again.

@brandur-stripe brandur-stripe merged commit 9afd73c into stripe:master Jan 10, 2020
@brandur-stripe
Copy link
Contributor

Just re-ran our suite on 2.7 — after we get one more fix from Webmock, we'll be fully Ruby 2.7 warning-free!

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

Successfully merging this pull request may close these issues.

2 participants