piotrmurach / tty-command

Execute shell commands with pretty output logging and capture stdout, stderr and exit status.
https://ttytoolkit.org
MIT License
400 stars 34 forks source link

fix Ruby 2.7 warning: Using the last argument as keyword parameters i… #56

Closed abhinavs closed 3 years ago

abhinavs commented 3 years ago

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

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 commented 3 years ago

Coverage Status

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

coveralls commented 3 years ago

Coverage Status

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

jrochkind commented 3 years ago

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 commented 3 years ago

Thanks, trying that.

piotrmurach commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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