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

initial idea for clarifying names of token -> token_dict and token_str #869

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jvanasco
Copy link
Contributor

Here is an initial idea to start addressing #856

Various functions accept a parameter with a name like "token", but the parameter might either be a String or a Dict depending on the function. Sometimes two similar functions will require different input types - for example save_bearer_token operates on a Dict, but validate_bearer_token operates on a string. To better clarify this, and aid in future typing, the names of these functions were updated to have a _str or _dict suffix. On several functions, the type of input was not identified in the docstring and a code audit was needed to determine.

This does not change the various attributes on oauthlib.common.Request - which do need to be better listed and typed.

This quick PR was done for initial feedback. I did not run this through any tests yet.

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

the name change scheme seems to be OK but need input from more maintainers. also the related tests also should be renamed I think

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