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

Support OAuth1 client wreck options #481

Merged
merged 1 commit into from Aug 26, 2021
Merged

Support OAuth1 client wreck options #481

merged 1 commit into from Aug 26, 2021

Conversation

hueniverse
Copy link
Contributor

Adds support for passing Wreck options to allow setting agent and other overrides.

@hueniverse hueniverse added the feature New functionality or improvement label Aug 20, 2021
Copy link
Member

@Nargonath Nargonath left a comment

Choose a reason for hiding this comment

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

Shouldn't we document that behavior in the options section of the API.md? Apart from that and my little comment, LGTM.

lib/oauth.js Show resolved Hide resolved
@hueniverse
Copy link
Contributor Author

The client is not documents AFAICT which is why I didn't add that new option.

@devinivy devinivy merged commit 3b846e6 into master Aug 26, 2021
@devinivy devinivy deleted the oauth1wreck branch August 26, 2021 04:09
@devinivy devinivy self-assigned this Aug 26, 2021
@devinivy devinivy added this to the 12.3.0 milestone Aug 26, 2021
@devinivy
Copy link
Member

This is published in 12.3.0. To clarify the previous comment, just want to note that this is not exposed publicly through the strategy since it is guarded by the strategy's validation: just through using the Client interface directly. I do think that it would be nice to sync these back up eventually by exposing it through the strategy with a test and docs.

@hueniverse
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants