fix semver not fully covered#1548
fix semver not fully covered#1548bearomorphism wants to merge 2 commits intocommitizen-tools:masterfrom
Conversation
bearomorphism
force-pushed
the
bear-fix-semver-not-fully-covered
branch
from
0972ba2 to
877fcd6
Compare
| (("1.0.0-reallyweird", "PATCH", "reallyweird", 0, None), "1.0.0-reallyweird1"), | ||
| (("v0.7.1-release", "PATCH", "release", 0, None), "0.7.1-release1"), | ||
| (("v0.0.1-SNAPSHOT", "PATCH", "SNAPSHOT", 0, None), "0.0.1-snapshot1"), |
There was a problem hiding this comment.
@Lee-W I'm not entirely sure if these test cases are correct. Could you help to review?
bearomorphism
force-pushed
the
bear-fix-semver-not-fully-covered
branch
from
877fcd6 to
da46193
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1548 +/- ##
==========================================
+ Coverage 97.33% 98.25% +0.91%
==========================================
Files 42 58 +16
Lines 2104 2696 +592
==========================================
+ Hits 2048 2649 +601
+ Misses 56 47 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bearomorphism
changed the title
| (("1.0.0-reallyweird", "PATCH", "reallyweird", 0, None), "1.0.0-reallyweird1"), | ||
| (("v0.7.1-release", "PATCH", "release", 0, None), "0.7.1-release1"), | ||
| (("v0.0.1-SNAPSHOT", "PATCH", "SNAPSHOT", 0, None), "0.0.1-snapshot1"), |
There was a problem hiding this comment.
| (("1.0.0-reallyweird", "PATCH", "reallyweird", 0, None), "1.0.0-reallyweird1"), | |
| (("v0.7.1-release", "PATCH", "release", 0, None), "0.7.1-release1"), | |
| (("v0.0.1-SNAPSHOT", "PATCH", "SNAPSHOT", 0, None), "0.0.1-snapshot1"), | |
| (("1.0.0-reallyweird", "PATCH", "reallyweird", 0, None), "1.0.1-reallyweird1"), | |
| (("v0.7.1-release", "PATCH", "release", 0, None), "0.7.2-release1"), | |
| (("v0.0.1-SNAPSHOT", "PATCH", "SNAPSHOT", 0, None), "0.0.2-snapshot1"), |
I feel they probably should be this instead?
There was a problem hiding this comment.
I need some time to figure out how to adjust the logic.
There was a problem hiding this comment.
Isn't the version number already appended at the end of the version string?
There was a problem hiding this comment.
@Lee-W a gentle reminder for this discussion
There was a problem hiding this comment.
also it might be a good idea to make these named tuple. too hard to read...
There was a problem hiding this comment.
I agree the tests can be more readable. I had #1598 that attempts to make the tests read better.
I can make the NamedTuple change in that PR.
|
|
||
| # See https://github.com/pypa/packaging/blob/14b83e15dbb9caa87c63646ba7808b2b5e460ce6/src/packaging/version.py#L117 | ||
| # TODO: add more test cases for this pattern | ||
| _VERSION_PATTERN = r"""^\s* |
There was a problem hiding this comment.
One thing to note is that Python does not use semver. It exists before semver
There was a problem hiding this comment.
Should I add any comments here?
There was a problem hiding this comment.
not a bad idea. more like a reminder that we can not fully depend on python packaging version
|
I will update this PR this week when I have bandwidth |
bearomorphism
force-pushed
the
bear-fix-semver-not-fully-covered
branch
from
da46193 to
3081fff
Compare
|
Maybe we can adjust the workflow. Several PRs got closed just because the target branch is deleted |
|
Yep, this was a temporary workflow. we usually don't receive this many PR and don't review this fast. Maybe worth rethink it to make a rc to avoid what we encountered yesterday as well |
bearomorphism
force-pushed
the
bear-fix-semver-not-fully-covered
branch
from
3081fff to
8ee1dbe
Compare
bearomorphism
force-pushed
the
bear-fix-semver-not-fully-covered
branch
from
8ee1dbe to
bb48144
Compare
| (?P<release>[0-9]+(?:\.[0-9]+)*) # release segment | ||
| (?P<pre> # pre-release | ||
| [-_\.]? | ||
| # BEGIN different from packaging.version.py |
There was a problem hiding this comment.
Added those comments.
2 participants
Description
Related issue: #950
Checklist