-
-
Notifications
You must be signed in to change notification settings - Fork 619
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
Add type hints for the resolver module #1316
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1316 +/- ##
==========================================
+ Coverage 99.58% 99.62% +0.03%
==========================================
Files 33 33
Lines 2908 2921 +13
Branches 321 318 -3
==========================================
+ Hits 2896 2910 +14
+ Misses 6 5 -1
Partials 6 6
Continue to review full report at Codecov.
|
piptools/resolver.py
Outdated
and self.specifier == other.specifier | ||
and self.extras == other.extras | ||
) | ||
return NotImplemented |
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.
FYI: this line left uncovered, so I've added it to exclude_lines
to the coverage report config in a619d0d
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 not add tests instead?
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.
Okay, let's add one.
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.
Added in cabb979.
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.
Most of the change is good but I've added a few comments to places that are rather problematic.
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.
Nice work! Glad to see the typing continue.
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
Co-Authored-By: Jon Dufresne <[email protected]>
Co-Authored-By: Jon Dufresne <[email protected]>
@jdufresne @webknjaz thanks for helping me with this PR! 🙏🏼 I've addressed all suggestions and I think it's ready for the second review round. |
Co-authored-by: Sviatoslav Sydorenko <[email protected]>
@jdufresne @webknjaz thanks for reviewing! 🙏🏼 |
Refs: #972
Additionally refactored
lookup_table()
:tests/test_utils.py
and add more tests