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 Ruby 2.7 warning: Using the last argument as keyword parameters i… #56

Merged
merged 1 commit into from
Jul 27, 2020

Conversation

abhinavs
Copy link
Contributor

Describe the change

Ruby 2.7 warning: "Using the last argument as keyword parameters is deprecated"

Why are we doing this?

In Ruby 2.7, the way ruby handles the positional arguments and keyword arguments is changed, aiming to provide a smooth transition to Ruby 3. This has resulted in warnings on the command line when one uses cmd.run.

Benefits

This will remove the warning message which is thrown

Drawbacks

  • Doesn't seem a very elegant way of doing this
  • I am not sure if there's a better way to add this or there are more places where this is needed.

Requirements

Put an X between brackets on each line if you have done the item:
[] Tests written & passing locally?
[] Code style checked?
[] Rebased with master branch?
[] Documentation updated?

@coveralls
Copy link

coveralls commented Jul 27, 2020

Coverage Status

Coverage remained the same at 94.786% when pulling ca68530 on abhinavs:master into e7fb8c3 on piotrmurach:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 93.939% when pulling 37cac2c on abhinavs:master into e7fb8c3 on piotrmurach:master.

@jrochkind
Copy link

jrochkind commented Jul 27, 2020

I believe the **@cmd_options version will work on rubies earlier than 2.7. I think it actually may work since ruby 2.0? Travis does test all the way back to 2.0 (which is further back than I expected!).

But it might be worth a try doing just the double splat version, and seeing if it passes CI? I think it may. So you may not need the ugly conditional, double splat can, I suspect, work in all supported versions. Alternately, if it does need a conditional, I think double-splat will work further back than 2.7.

@abhinavs
Copy link
Contributor Author

Thanks, trying that.

@piotrmurach piotrmurach merged commit dab9650 into piotrmurach:master Jul 27, 2020
@piotrmurach
Copy link
Owner

@jrochkind Thanks for the review and helping out! ❤️

@abhinavs Thanks for fixing the warnings. ❤️

I'm currently going through tty gems and updating them one by one. One thing I'm changing is swapping hash options to named keyword arguments. I plan to do the same here.

@abhinavs
Copy link
Contributor Author

no problem, in fact, thanks a lot for creating tty and tty plugins - design is very clean. Just created blockr - used tty, tty-file and tty-command to create it. I am still exploring, will be building a few more command line apps.

@piotrmurach
Copy link
Owner

@abhinavs blockr sounds great! I enjoy hearing about new command-line tools built with tty components. Looking forward to more projects. A small heads-up, there is a road map for the tty project in which I plan to drop thor and replace it with a more powerful command-line parsing gem tty-option. You could explore it in your future projects as well and see how you like it.

@abhinavs
Copy link
Contributor Author

looks very intuitive and very readable - will definitely try it out. Thanks @piotrmurach

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.

4 participants