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

[RUBY] Replace deprecated URI.encode to support ruby 2.7 without warnings #10445

Merged
merged 4 commits into from
Sep 2, 2020

Conversation

kitop
Copy link
Contributor

@kitop kitop commented Sep 1, 2020

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Issue: #10444

Ruby's URI.encode has been deprecated over 10 years ago. In addition, Ruby 2.7 now shows the warning in non-verbose mode, resulting in very noisy logs.

This tackles the problem via the second solution proposed in the issue.
It adds a dependency on the addressable gem, and uses Addressable::URI.encode which is part-aware.

Already tried this in a generated library with success: https://github.com/netlify/zuora-ruby-client/pull/7/files

/cc @zlx

@kitop
Copy link
Contributor Author

kitop commented Sep 1, 2020

Tests are failing due to un unrelated change in Typhoeus: typhoeus/typhoeus#575
Typhoeus::Response::Header no longer inherits from Hash, but now is a DelegateClass(Hash). And since it's checking the type in https://github.com/kitop/swagger-codegen/blob/5236247dce2ac814a71a031763aa78959d42b424/samples/client/petstore/ruby/spec/pet_spec.rb#L82 it fails.

An option would be to change that check, or another one, to call to_h here: https://github.com/kitop/swagger-codegen/blob/5236247dce2ac814a71a031763aa78959d42b424/samples/client/petstore/ruby/lib/petstore/api_client.rb#L66

Thoughts?

@kitop
Copy link
Contributor Author

kitop commented Sep 1, 2020

Pushed the second option to have the test passing. Let me know if you'd prefer an alternative, or a separate PR.

@HugoMario
Copy link
Contributor

Thanks a lot for PR @kitop!! Here is fine :)

@HugoMario HugoMario merged commit 1ffcd68 into swagger-api:master Sep 2, 2020
This was referenced Mar 11, 2021
This was referenced Mar 11, 2021
fotos added a commit to wimit/wimit-swagger-codegen that referenced this pull request Jun 5, 2022
fotos added a commit to wimit/wimit-swagger-codegen that referenced this pull request Jun 5, 2022
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.

None yet

2 participants