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 URI.encode deprecation warnings for Ruby 3 compatibility #73

Merged
merged 9 commits into from
Jan 13, 2022

Conversation

tjvman
Copy link
Member

@tjvman tjvman commented Jan 6, 2022

Unit tests passed against Ruby 2.7.2 and Ruby 3.0.3.

We chose to use the Addressable gem rather than the stdlib's CGI.escape
method because Addressable conforms better to the RFC-3896 spec, which
was the issue with URI.encode.

Co-authored-by: Seth Boyles <sboyles@pivotal.io>
Co-authored-by: Tom Viehman <tviehman@pivotal.io>
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/180829543

The labels on this github issue will be updated when the story is started.

@sethboyles
Copy link
Member

This addresses #68

@bgandon
Copy link
Contributor

bgandon commented Jan 7, 2022

I've pushed some more adjustments in order to remove warnings when running unit tests.
Tested successfully with Ruby 3.1.0, 2.7.2 and 3.0.2.

@strehle, could you please review and merge this PR?

@bgandon bgandon self-requested a review January 7, 2022 17:25
@tjvman
Copy link
Member Author

tjvman commented Jan 7, 2022

@bgandon @strehle If/when this PR is merged, do y'all plan to cut a release of the gem?

We're planning to upgrade to Ruby 3, and we want to have a plan for how we consume this gem - through RubyGems release, targeting master (not sure how stable master is meant to be), or some other method.

@strehle
Copy link
Member

strehle commented Jan 8, 2022

I can merge - no matter, but I cannot release to rubygems, so @peterhaochen47 @bruce-ricard either you have the key to do a release or do you know where in pipeline there is a ship-it ?
I see only cf-uaac (command line tool itself) pipeline , e.g. https://hush-house.pivotal.io/teams/cf-uaa/pipelines/cf-uaac

I will join next WG status meeting on thursday, but we need some colleague(s) from vmware

@bgandon bgandon requested review from strehle and removed request for strehle January 8, 2022 14:41
@bgandon
Copy link
Contributor

bgandon commented Jan 8, 2022

@tjvman I think Markus has covered the organisational challenge around publishing a new version of this Gem. This is certainly still in the hands of VMware even though other members of the CFF Working Group like SAP (which I'm part of) should also have the ability to cut new releases.

Anyway, cutting a new version of this Gem is a major step in order to get proper Ruby 3 support into cf-uaac, which will be a great achievement! 😃

And we are very close to it, as I've pushed the tested code in cloudfoundry/cf-uaac#95.

.travis.yml Outdated Show resolved Hide resolved
Copy link
Member

@strehle strehle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment, rest ok

@strehle
Copy link
Member

strehle commented Jan 12, 2022

@bgandon can you please resolve conflicts and rebase maybe you use githubActions .
We can discuss #78 in next WG meeting

@strehle strehle merged commit c171e4b into master Jan 13, 2022
@strehle strehle deleted the use-addressable-uri-encode branch January 13, 2022 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

"URI.escape is obsolete" warning displays in ruby 2.7
7 participants