-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
add new variable to default unset params #1557
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
Closed
jkklapp
wants to merge
8
commits into
encode:master
from
jkklapp:use_better_variable_name_in_default_param
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
429c139
add new variable to default unset params
jkklapp 6c915bd
Merge branch 'master' into use_better_variable_name_in_default_param
jkklapp 02bc561
import variable in __init__
jkklapp 08e0c50
use ClientDefaultType over UnsetType
jkklapp 0d9463c
Merge branch 'master' into use_better_variable_name_in_default_param
lovelydinosaur 75c335b
Merge branch 'master' into use_better_variable_name_in_default_param
lovelydinosaur 5e8bd6a
revert typing change for Timeout class
jkklapp c97e220
Merge branch 'master' into use_better_variable_name_in_default_param
lovelydinosaur File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, these cases in
_client.pyshould be:(But the signature of the arguments in the Timeout class should keep using
UNSET)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually thinking about this, it's probably also a good idea to move
ClientDefaultTypeandUSE_CLIENT_DEFAULTout of_types.py, and into this module, to keep them close to where they're actually used.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I do that I need to also change the the types in the
Timeoutclass, otherwisemypycomplains.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll take a look.