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 deprecated usage of Kernel#open to open URIs #27

Closed

Conversation

Lavode
Copy link

@Lavode Lavode commented Nov 1, 2020

Ruby 2.7 deprecated the usage of Kernel#open to open URIs directly,
leading to a deprecation warning and an argument error due to
Kernel#open not handling more than one argument.

You can reproduce the error as follows:

[4] pry(main)> require 'steam-condenser';
[5] pry(main)> ::WebApi.api_key = '*snip*';
[6] pry(main)> SteamId.resolve_vanity_url('morrolan-lz');
/home/michael/.gem/ruby/2.7.2/gems/steam-condenser-1.3.11/lib/steam/community/web_api.rb:130: warning: calling URI.open via Kernel#open is deprecated, call URI.open directly or use URI#open
ArgumentError: extra arguments
from /opt/rubies/ruby-2.7.2/lib/ruby/2.7.0/open-uri.rb:153:in `open_uri'

Ruby 2.7 deprecated the usage of `Kernel#open` to open URIs directly,
leading to a deprecation warning and an argument error due to
`Kernel#open` not handling more than one argument.
@Lavode
Copy link
Author

Lavode commented Nov 1, 2020

Heh, seems URI::open was private up to Ruby 2.7. At the same time, Ruby 2.7's Kernel#open hook by OpenURI seems to not accept arbitrary arguments (for HTTP headers) anymore, throwing the ArgumentError mentioned above.
Seems like a bit of a peculiar way to deprecate this feature, as it does not provide a clean version-agnostic way to have code run on both 2.7 and older versions.

@Arie
Copy link
Contributor

Arie commented May 18, 2021

@Lavode I didn't see your PR earlier, but I've made a separate one that I've tested on 2.5.9, 2.7.3 and 3.0.1 (also ran resolve_vanity_url) by wrapping the open call in a separate module based on the RUBY_VERSION.
#28

@koraktor
Copy link
Owner

Sorry @Lavode for not having the time looking into this PR.

@Arie made a great job fixing the very same problem in #28 and I'll merge this later.

Thank you, nevertheless.

@koraktor koraktor closed this May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants