Skip to content

Add missing constants to flask-cors stubs - #12585

Merged
srittau merged 1 commit into
python:mainfrom
ashm-dev:dev
Aug 24, 2024
Merged

srittau merged 1 commit into
python:mainfrom
ashm-dev:dev

Conversation

@ashm-dev

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood changed the title Add stubs for flask-cors Add missing constants to flask-cors stubs Aug 24, 2024

@srittau srittau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

(At some point, the constants could be converted to Final strings that are assigned the actual value, but that can wait for another PR.)

@srittau
srittau merged commit c54f16b into python:main Aug 24, 2024
@ashm-dev
ashm-dev deleted the dev branch August 24, 2024 18:58
@ashm-dev

Copy link
Copy Markdown
Contributor Author

Thanks!

(At some point, the constants could be converted to Final strings that are assigned the actual value, but that can wait for another PR.)

Thanks!
Do you mean for me to pack them in Final? I was just taking an example from other packages, so I did it the way it was)

@srittau

srittau commented Aug 25, 2024

Copy link
Copy Markdown
Collaborator

The current gold standard for constants is to use Final and the actual constant value (if it is relevant) or use Final with the type if the constant value is not really relevant:

ANSWER_QUESTION_OF_LIFE: Final = 42
SOME_RANDOM_CONSTANT: Final[int]

We'd certainly appreciate a PR changing the flask-cors constants accordingly.

@ashm-dev

Copy link
Copy Markdown
Contributor Author

@srittau Thanks for the explanation, I'll try to update soon!)

max-muoto pushed a commit to max-muoto/typeshed that referenced this pull request Sep 8, 2024
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.

2 participants