-
-
Notifications
You must be signed in to change notification settings - Fork 950
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
Annotate samesite
parameter on set_cookie
#1590
Conversation
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.
probably weird question but shouldn't it be Lax
Strict
and None
or we lowercase everything and it doesnt make a difference ?
Good question. We lower case anyway on Maybe the right type would be |
I think this is good. It helps users but also developers so we can know what values are accepted just by looking at it. I think we should just type it all as lowercase, even if at runtime we accept both. Side note: would it make sense to have a _typing.py module for this sort of thing? |
Not sure... Just for the type added here is enough? 🤔 Maybe... Is it the only type that could be added there right now? 🤔 In any case, I guess is not a blocker for this PR. I'll wait to see if someone has any other comment about this PR, and merge it before 0.20.0. |
Just for this it doesn't make sense, but using that argument it would never make sense. I think there are other shared things like this scattered around the codebase? And maybe some of the stuff in typing.py and datastructructures.py could go in there? Just a thought... |
No description provided.