-
Notifications
You must be signed in to change notification settings - Fork 96
First implementation of next_version #222
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
First implementation of next_version #222
Conversation
dee4dff
to
55d63b0
Compare
e43fa78
to
5dcc093
Compare
@tomschr here are a few more (currently failing) test cases:
|
Open issues:
Currently there is not enough information to determine which of the three alternatives to choose. It needs either an extra parameter, or since this affects only prereleases, allow two more
What do you think? |
5dcc093
to
99d7660
Compare
I've added them to the test file and fixed them. Additionally, I've added you as co-author. The implementation currently looks horrible as it contains a lot of |
Getting closer :) Four more failing cases:
|
Here's a simplified implementation that passes all 12 cases, feel free to apply the diff: diff --git a/semver.py b/semver.py
index 67ddded..294d369 100644
--- a/semver.py
+++ b/semver.py
@@ -715,24 +715,19 @@ def next_version(version, part, prerelease_token="rc"):
)
)
version = VersionInfo.parse(version)
- if part == "major" and version.prerelease:
- return format_version(version.major, version.minor, version.patch)
- elif part == "major" and not version.prerelease:
- return str(version.bump_major())
- elif part == "minor" and version.prerelease:
- return format_version(version.major, version.minor, version.patch)
- elif part == "minor" and not version.prerelease:
- return str(version.bump_minor())
- elif part == "patch" and version.prerelease:
- return format_version(version.major, version.minor, version.patch)
- elif part == "patch" and not version.prerelease:
- return str(version.bump_patch())
- elif part == "prerelease" and version.prerelease:
- return str(version.bump_prerelease(token=prerelease_token))
- elif part == "prerelease" and not version.prerelease:
- return str(version.bump_patch().bump_prerelease(token=prerelease_token))
-
- raise RuntimeError("Should not happen, got {part}".format(part=part))
+ if version.prerelease and (
+ part == "patch"
+ or (part == "minor" and version.patch == 0)
+ or (part == "major" and version.minor == version.patch == 0)
+ ):
+ return str(version.replace(prerelease=None))
+
+ if part in ("major", "minor", "patch"):
+ return str(getattr(version, "bump_" + part)())
+
+ if not version.prerelease:
+ version = version.bump_patch()
+ return str(version.bump_prerelease(prerelease_token))
# ---- CLI |
99d7660
to
171fc97
Compare
I'm wondering, if we should check parts specifically for diff --git i/semver.py w/semver.py
index b6443ae..5e69b1b 100644
--- i/semver.py
+++ w/semver.py
@@ -708,6 +708,8 @@ def next_version(version, part, prerelease_token="rc"):
"prerelease",
# "build", # ???
}
+ if part == "build":
+ raise ValueError("The build part is not predictable") I hadn't had a better idea for the error message. What do you think/prefer? |
I don't mind the generic |
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.
LGTM
@gsakkis Ok, I have to think again what would be better. Do you have anything else which needs to be covered in this function? If not, I would implement the rest and extend the documentation. It would be very helpful if you could review it. 😉 |
c787054
to
f3aa4ab
Compare
@gsakkis George, during documenting the function, I found something where I need your help. Consider the following code example: >>> semver.next_version("3.4.5-pre.2+build.4", part="prerelease") # (1)
'3.4.5-pre.3'
>>> semver.next_version("3.4.5-pre.2+build.4", part="patch") # (2)
'3.4.5+build.4
>>> semver.next_version("3.4.5+build.4", part="patch") # (3)
'3.4.6' The first one is plausible, but what about the second one? Shouldn't it be I could fix and remove the build part when I patch your version in the following lines: diff --git i/semver.py w/semver.py
index ac3bfd1..48241ed 100644
--- i/semver.py
+++ w/semver.py
@@ -728,12 +728,12 @@ def next_version(version, part, prerelease_token="rc"):
)
)
version = VersionInfo.parse(version)
- if version.prerelease and (
+ if (version.prerelease or version.build) and (
part == "patch"
or (part == "minor" and version.patch == 0)
or (part == "major" and version.minor == version.patch == 0)
):
- return str(version.replace(prerelease=None))
+ return str(version.replace(prerelease=None, build=None)) Would that be plausible for you? What do you think? (You can look at the latest commit where I've adjusted the tests and applied the diff from above). I guess, we run into these uncharted area of the build part. 😉 |
af7e722
to
56864d1
Compare
Indeed, afaict the rules regarding the build part are fuzzy but your changes look reasonable, at least more reasonable than preserving the build when bumping, so |
@gsakkis I just remembered, we should probably integrate the functionality into the CLI script |
b71d1de
to
3c3388d
Compare
I've added now the new subcommand $ pysemver nextver 1.2.3 prerelease
1.2.4-rc.1 |
@gsakkis However, I would avoid a module level function My proposal would be: def next_version(self, part:str, prerelease_token:str="rc"):
"""
Determines the next version, taking prereleases into account.
The "major", "minor", and "patch" raises the respective parts like
the ``bump_*`` functions. The real difference is using the
"preprelease" part. It gives you the next patch version of the prerelease,
for example:
>>> str(semver.VersionInfo.parse("0.1.4").next_version("prerelease"))
'0.1.5-rc.1'
:param part: One of "major", "minor", "patch", or "prerelease"
:param prerelease_token: prefix string of prerelease, defaults to 'rc'
:return: a new VersionInfo instance, it doesn't modify self
"""
validparts = frozenset([
"major",
"minor",
"patch",
"prerelease",
])
cls = type(self)
if part not in validparts:
raise ValueError(
"Invalid part. Expected one of {validparts}, but got {part!r}".format(
validparts=validparts, part=part
)
)
if (self.prerelease or self.build) and (
part == "patch"
or (part == "minor" and self.patch == 0)
or (part == "major" and self.minor == self.patch == 0)
):
newver = cls(*self.to_tuple())
return newver.replace(prerelease=None, build=None)
if part in ("major", "minor", "patch"):
return getattr(self, "bump_" + part)()
version = self
if not self.prerelease:
version = self.bump_patch()
return version.bump_prerelease(prerelease_token) I haven't tested it, but I thought, I write it down to document it. |
Yes absolutely, it makes sense. |
@gsakkis I've tried to rebase and adjust the changes. However, Python2 emits this error: $ tox -e py27
[...]
def iter_versions(self, firstpart, *parts, prerelease_token="rc"):
^
SyntaxError: invalid syntax One solution would be to add this for Python3 only and leave Python2 behind. What would you think? |
3c3388d
to
85d4ff1
Compare
Ok, it seems I introduced something like |
85d4ff1
to
477e8f2
Compare
@gsakkis I've fixed the conflicts and implemented the I was wrong, it's not a Python3 only issue, so I removed this label. Maybe we could even incorporate it into the 2.10.0 release. Locally, all tests of Python 2.7 and 3.6 were successful. However, it would be good to have a final look and check it. That would be great, George! 👍 |
477e8f2
to
6c48f6d
Compare
6c48f6d
to
c0a1f14
Compare
Synopsis: semver.VersionInfo.next_version(version, part, prerelease_token="rc") * Add test cases * test_next_version * test_next_version_with_invalid_parts * Reformat code with black * Document it in usage.rst * Implement "nextver" subcommand for pysemver command Co-authored-by: George Sakkis <gsakkis@users.noreply.github.com> Co-authored-By: Karol <ppkt@users.noreply.github.com>
c0a1f14
to
5b04960
Compare
@gsakkis If you don't mind or object, I'd like to merge it soon (Saturday?) to make the 2.10.0 release. 😄 |
@tomschr LGTM! |
This PR contains (based on #221):
TODOs:
semver.VersionInfo.next_version
counterpart@gsakkis This is a first draft, feel free to comment. 😉