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

Deprecate QParams validation so that we can later remove it. #274

Merged
merged 11 commits into from
Nov 12, 2014

Conversation

n8han
Copy link
Member

@n8han n8han commented Oct 19, 2014

This is used in the oauth and mac modules. It might be better to port
those over to Directives in the same PR, as they generate a lot of
deprecation warnings. Let's discuss before merging.

Nathan Hamblen added 2 commits October 19, 2014 14:48
This is used in the oauth and mac modules. It might be better to port
those over to Directives in the same PR, as they generate a lot of
deprecation warnings. Let's discuss before merging.
Since the error messages generated were just being thrown away, we can
instead just `flatMap` through options.
@chrislewis
Copy link
Member

The important part is that the conversion happens before this is released, and while I like the discreteness of this pull vs another to upgrade those modules (as one does not immediately necessitate the other), if you're feeling inspired ...

@n8han
Copy link
Member Author

n8han commented Oct 19, 2014

Yeah, I'm concerned about not getting it done otherwise. I did the easy one, I'll take a crack at the harder one.

@n8han n8han added this to the 0.8.3 milestone Oct 19, 2014
@n8han
Copy link
Member Author

n8han commented Oct 20, 2014

Finished with unfiltered-oauth, all that's left is unfiltered-oauth2. 😴

@n8han n8han self-assigned this Oct 20, 2014
Nathan Hamblen added 4 commits October 20, 2014 22:22
This is a blunt translation of QParams code to directives, it could
probably benefit from a comprehensive rewrite at some point. My aim
was to preserve the exiting behavior.

The tests pass.
@n8han
Copy link
Member Author

n8han commented Oct 27, 2014

OK, I've replaced the use of QParams with parameter directives in both the oauth 1 and 2 integration libraries. Their automated tests pass, but since this a substantial change I would feel better if we tested it with an implementing server. @softprops would you have a suggestion there?

n8han pushed a commit that referenced this pull request Nov 12, 2014
Deprecate `QParams` validation so that we can later remove it.
@n8han n8han merged commit 8ec6b89 into 0.8.3 Nov 12, 2014
@xuwei-k xuwei-k deleted the deprecate_qparams branch December 3, 2016 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants