Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2349 +/- ##
==========================================
+ Coverage 99.12% 99.16% +0.03%
==========================================
Files 40 40
Lines 3098 3098
Branches 788 680 -108
==========================================
+ Hits 3071 3072 +1
Misses 15 15
+ Partials 12 11 -1 |
cff1fdb to
23b746c
Compare
23b746c to
0225875
Compare
| build-backend = "hatchling.build" | ||
|
|
||
| [dependency-groups] | ||
| dev = [ |
There was a problem hiding this comment.
What is the difference between this and the dev optional dependencies?
There was a problem hiding this comment.
I did some mess in the conversion, but fixed.
[dependency-groups] are the correct part for dev dependencies, the optional dependencies are the extras.
| import os | ||
| import sys | ||
| from typing import Any, Dict, Iterator, List | ||
| from typing import Any, Iterator |
There was a problem hiding this comment.
These changes seem unrelated? Why are we removing user_options ?
There was a problem hiding this comment.
A bit, but for some reason mypy started complaining about it. It may be it happened because mypy got upgraded on the way.
There was a problem hiding this comment.
Should we at least make a small mention of it in the changelog?
I'm not too up to speed with what this does, but I can assume users can be surprised this is removed without a notice.
There was a problem hiding this comment.
@DanielNoord I actually returned with user_options to be extra safe and put a type ignore with a comment so mypy does not complain.
DanielNoord
left a comment
There was a problem hiding this comment.
Only one comment about the removal. Rest LGTM!
| import os | ||
| import sys | ||
| from typing import Any, Dict, Iterator, List | ||
| from typing import Any, Iterator |
There was a problem hiding this comment.
Should we at least make a small mention of it in the changelog?
I'm not too up to speed with what this does, but I can assume users can be surprised this is removed without a notice.
| description = "Run isort on modules registered in setuptools" | ||
| user_options: List[Any] = [] | ||
| # Potentially unused variable - check if can be safely removed | ||
| user_options: list[Any] = [] # type: ignore |
There was a problem hiding this comment.
| user_options: list[Any] = [] # type: ignore | |
| user_options: list[Any] = [] # type: ignore[error-code] |
Nit, but I always add the error code that I'm ignoring so that I don't accidentally ignore a new error code after a mypy bump. Could you add the specific code?
04cf004 to
7f44d60
Compare
7f44d60 to
16bb0e2
Compare
DanielNoord
left a comment
There was a problem hiding this comment.
Awesome! Nice job on doing this migration 🚀
Thanks a lot, nice job on review also! |
| @@ -24,18 +24,13 @@ jobs: | |||
| python-version: ${{ matrix.python-version }} | |||
| cache: "pip" | |||
There was a problem hiding this comment.
By the way, I think these caches are now outdated.
Maybe something for a follow up though!
Reverts #2347
Closes #2337