-
Notifications
You must be signed in to change notification settings - Fork 1
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
Utilize packaging.version.Version
#220
Conversation
Implementing digestion of packaging.version.Version instances in SemanticVersion to be used when updating version specifiers, but still have the usability of the special SemanticVersion methods.
Implement support for packaging.version.Version in SemanticVersion, making it able to parse its current internals as a packaging.version.Version, as well as being able to digest one. Add epochs to versions if necessary.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #220 +/- ##
==========================================
+ Coverage 80.07% 80.85% +0.78%
==========================================
Files 12 12
Lines 838 888 +50
==========================================
+ Hits 671 718 +47
- Misses 167 170 +3 ☔ View full report in Codecov by Sentry. |
Overall general comments:
Specific Suggestions:
|
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.
I think this code probably introduces more complexity than it needs to, missing test coverage is also rather worrysome. I think some of the functions need to be refactored a bit mroe
The missing test coverage I'd consider quite minimal here (see the codecov report comment above). However, yes, in general (meaning prior to this PR) the test coverage of parts in Unfortunately, the whole thing is quite complex, and there's a lot of added complexity due to a lot of error handling. |
Concerning the curated AI overview, I have addressed all of these points, except the first one: refactoring |
Co-authored-by: Daniel Marchand <[email protected]>
To follow up on this, I have opened #222. |
Fixes #221
Since we are currently only supporting Python packages, it makes sense to use the
packaging.version.Version
class to parse the latest retrieved version from PyPI. This ensures we can handle the quirks of Python versions.Addresses the bold parts of #217, and the immediate bug there.
Specifically, this addresses the
epoch
andpost-release
segments, adding tests for these as well.For now, all non
base_version
segments are stripped from the version, meaning only epoch (if present) + release (major.minor.patch) are included from a parsedVersion
instance.Extra logic is needed to handle and support the remaining
Version
segments, as is also mentioned in #217.