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

Require plaintext signature method #135

Merged
merged 3 commits into from Nov 1, 2021
Merged

Require plaintext signature method #135

merged 3 commits into from Nov 1, 2021

Conversation

confiks
Copy link
Contributor

@confiks confiks commented Aug 8, 2016

For some reason the plaintext signature class is not require'd by default, and cannot be used without explicitly requiring it. This has historically been so, and the tests go out of their way to manually require the class: https://github.com/oauth-xx/oauth-ruby/blob/705ae93/test/units/test_net_http_client.rb#L28. I have no idea why this choice has been made.

Last year in issue #76 some users bumped their head against this. This should either be documented, or the class should just be required.

This pull request adds the require statement and removes all the require statements from the tests.

I also modified travis.yml to use a specific rubygems version, because master is probably broken (rubygems/rubygems#1679) and it can break in the future. The Travis CI default (v2.5.1) unfortunately is broken as well (rubygems/rubygems#1223).

@mpapis
Copy link
Member

mpapis commented Aug 8, 2016

@thejamespinto LGTM, second pair of eyes?

@kolkheang
Copy link

+1. Thank you @confiks!

@Calamitous
Copy link

+1 to this; we're having the same issue.

Copy link
Member

@pboling pboling left a comment

Choose a reason for hiding this comment

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

This looks great, but conflicts need to be resolved.

@confiks
Copy link
Contributor Author

confiks commented Jan 28, 2021

I'm not going to do this after more than four years.

@confiks confiks closed this Jan 28, 2021
@pboling
Copy link
Member

pboling commented Nov 1, 2021

That's quite reasonable! New management is going to handle it still. ;)

@pboling pboling reopened this Nov 1, 2021
@pboling pboling merged commit 6aad073 into oauth-xx:master Nov 1, 2021
@confiks
Copy link
Contributor Author

confiks commented Nov 1, 2021

Nice 👍

Please keep in mind that this PR included a explicit version choice of rubygems v2.6.6, which by now is quite ancient and probably unnecessary.

@confiks
Copy link
Contributor Author

confiks commented Nov 1, 2021

Ah, but you removed .travis.yml entirely, so all good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants