-
Notifications
You must be signed in to change notification settings - Fork 147
pyproject.toml: add new poe target "types-extra" #875
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: main
Are you sure you want to change the base?
Conversation
As kindly suggested by the existing "types" target: $ uv run poe types Poe => mypy --package west src/west/app/project.py:1364: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] Do not include in the "all" group because it's not passing. Signed-off-by: Marc Herbert <[email protected]>
This was the only --check-untyped-defs warning in manifest.py: easy win. Signed-off-by: Marc Herbert <[email protected]>
| def get(self, option, **kwargs): | ||
| def get( | ||
| self, option: str, default: str | None = None, configfile: ConfigFile = ConfigFile.ALL | ||
| ) -> str | None: |
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.
This is a public, documented method so it's not going to change any time soon.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #875 +/- ##
=======================================
Coverage 84.40% 84.40%
=======================================
Files 11 11
Lines 3385 3385
=======================================
Hits 2857 2857
Misses 528 528
|
|
Current output cc: @thorsten-klein |
|
Then, maybe we could run this failing checkin in CI with a Similarly to 361004d? I forgot how that went. Either way this can be merged first. |
|
Why not enabling I will start fixing the issues on main👍 |
I'd also like it if the issues were dealt with, and we can simply enable the untyped checks in the |
More importantly, because:
That's why this PR is not adding anything to CI. If you want the extra, unfixable warnings then it's now very easy to see them. If you don't, then just keep ignoring as you've been doing before. The same "extra" logic could apply to other options than --check-untyped-defs - even if all --check-untyped-defs turn out to be fixable. BTW there are --check-untyped-defs warnings in only three files (the bigger and more complex ones), so it's easy to spot warnings in other files. That's one value of |
Great but please avoid git conflicts with PRs currently in progress (unless it's an actual bug and not just a warning). EDIT, see: |
As kindly suggested by the existing "types" target: