JEG2 / highline

A higher level command-line oriented interface.
Other
1.29k stars 137 forks source link

Avoid using the last argument as keyword parameter #248

Closed LinusU closed 1 year ago

LinusU commented 3 years ago

This fixes the following warning when running the code with Ruby 2.7:

gems/highline-1.7.10/lib/highline.rb:624: warning: Using the last argument as keyword parameters is deprecated

Fastlane is still using the 1.x version of this library, and this warning is printed as soon as it's launched. It think it would be great if this could be released as a patch release (e.g. 1.7.11) so that the warning is no longer printed:

Screenshot 2021-04-14 at 14 14 24
ainame commented 3 years ago

@LinusU 👋 from fastlane core team, who is in charge of Ruby 3.0 support. Thank you for looking into this issue in fastlane! I've been recognising this and will be working on the migration soon.

That said, it doesn't prevent the gem owner from releasing a patched version though. I'll leave this PR.

abinoam commented 3 years ago

Hi @LinusU,

Thank you very much for your time spent on this.

Do you mind if I postpone the decision to merge and release it?

I've had no plan to release from 1-7-stable unless it would be a security patch, which is not the case.

I'm more prone to help people with this kind of problem to migrate to HighLine 2.

So, if @ainame has any problem migrating to HighLine 2 I would be available to help him (and any other open source maintainer) to do so.

Thank you again.

LinusU commented 3 years ago

@abinoam sure thing, no stress 👍

It seems like Fastlane is moving to 2.x sooner rather than later so this would be resolved then

ainame commented 3 years ago

@abinoam Thank you very much for your offer🙇

I have a question in migration 1.7.2 -> 2.0.3. I've checked the changelog file but it was a bit unclear to me that what is actually breaking changes. From what I see on the changelog, this is the only breaking API change between 1.7.2 and 2.0.0, isn't this?

PR #214 - Remove $terminal (global variable) (@abinoam)

fastlane support Ruby from 2.4 or newer so the changes in support Ruby version/environment wouldn't matter for us.

(fastlane also depends on commander gem that uses highline and they just fixed above https://github.com/commander-rb/commander/pull/73)

abinoam commented 3 years ago

Hi @ainame,

You're right. We have made our best effort to maintain near 100% percent compatibility as HighLine is a really old piece of software with a lot of other libs depending on it.

We have removed support to global variables as they are inherently evil most of the time. So the recommended way of use is to instantiate HighLine, save it somewhere and use from there without polluting the global namespace.

We have created 'highline/import' so if you consciously want to inject the HighLine methods into Kernel so they are globally available. The result would be probably 100% the same as using the old version of HighLine.

require 'highline'

# Basic usage

cli = HighLine.new
answer = cli.ask "What do you think?"
puts "You have answered: #{answer}"
require 'highline/import'

say "Now you can use #say directly"

Most people reported that the transition was smoothly no matter which way you choose.

Some other changes are related to "internal" implementation. So if the users' code rely on that level of detail it may fail when upgrading.

ainame commented 3 years ago

You're right. We have made our best effort to maintain near 100% percent compatibility as HighLine is a really old piece of software with a lot of other libs depending on it.

That sounds amazing! I really respect and appricate all of your works on that effort. As far as I checked fastlane doesn't use $terminal but used require 'highline/import' so I will look into a way to get rid of global references🙂

I will create another ticket if I get any issues in our migration🙇 Thank you very much.