-
-
Couldn't load subscription status.
- Fork 1.5k
Make uv run --all-extras pyright --verifytypes click pass
#3072
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
base: stable
Are you sure you want to change the base?
Conversation
I am not familiar with the CI setup of this project, so if there is a wish to have this in CI, I'd like to be instructed, or leave that to someone else please.
0c76f44 to
6e4585d
Compare
|
Please rebase and retarget to stable. |
|
Lines 171 to 176 in 81a482f
This is the tox job that runs type checks. Add the verifytypes call as another item in the list. |
|
Thank you @davidism , I have made the changes you requested |
pyproject.toml
Outdated
| dependency_groups = ["typing"] | ||
| commands = [ | ||
| ["mypy"], | ||
| ["pyright", "--verifytypes", "click"], |
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.
Need --ignoreexternal here as well.
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.
Why? Given that this is passing as-is
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.
Consistency with the other projects. Click doesn't have any dependencies (except stdlib modules), but if it did for some reason (like Flask) verifytypes tends to fail unhelpfully.
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.
Thank you. I have made this change.
I personally like to avoid --ignoreexternal so that I know the interfaces I ship are type safe, even if that means choosing to avoid some dependencies.
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.
For example, Flask-SQLAlchemy-Lite fails without ignoreexternal, even though SQLAlchemy goes to amazing lengths to work with typing.
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.
Thank you for the context. The latest commit addresses this so I'm hoping that it can be merged.
|
@davidism I have addressed all the inline comments and I am hoping for this to be merged. |
I am not familiar with the CI setup of this project, so if there is a wish to have this in CI, I'd like to be instructed, or leave that to someone else please.
fixes #3067