-
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
Support basic pre-release versions #31
base: master
Are you sure you want to change the base?
Conversation
Preserve any non-digit text at the end of the version string, since this frequently indicates a pre-release version like '1.0a1' or '1.0rc3'. This is the bare minimum to make pre-release versions work.
@@ -96,7 +96,7 @@ def get_git_version(path): | |||
# Short hash of the latest commit | |||
short_commit_name = commits[0].partition(' ')[0] | |||
# A valid version is sequence of dotted numbers optionally prefixed by 'v' |
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.
Comment is out of date.
# It is a release if current commit has a version tag (and dir is clean) | ||
release = (commit == commits[0]) and not dirty | ||
if not release: | ||
# We are working towards the next (minor) release according to PEP 440 | ||
version_numbers[-1] += 1 | ||
version = '.'.join([str(v) for v in version_numbers]) | ||
version = '.'.join([str(v) for v in version_numbers]) + prerelease |
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 don't think prerelease should be added if this is not a release. For example, after releasing 1.2b2, this would make the version for the next commit be 1.3b2.dev1+...
@@ -96,7 +96,7 @@ def get_git_version(path): | |||
# Short hash of the latest commit | |||
short_commit_name = commits[0].partition(' ')[0] | |||
# A valid version is sequence of dotted numbers optionally prefixed by 'v' | |||
valid_version = re.compile(r'^v?([\.\d]+)$') | |||
valid_version = re.compile(r'^v?(\d[\.\d]*)(.*)$') |
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.
Have you considered using the packaging
package to parse package versions? It will handle all the vagaries of PEP 440 including the assorted normalisations. It even deals with the leading v 😄. The only catch is it doesn't seem to have an easy way to manipulate the version (to bump it to the next one), so you'll need to reassemble all the components into a string.
If not, at least ensure that a dot at the end of the numeric version (e.g. 1.2.post1
) isn't consumed into the release portion regex. Something like
valid_version = re.compile(r'^v?(\d[\.\d]*)(.*)$') | |
valid_version = re.compile(r'^v?(\d+(?:\.\d+)*)(.*)$') |
would probably be more reliable (although it will still go wrong if someone uses epochs).
# Walk back along branch and find first valid tagged version (or use 0.0) | ||
for commit in commits: | ||
version_numbers = tagged_version(commit) | ||
version_numbers, prerelease = tagged_version(commit) |
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.
prerelease isn't really the right word - it could also be .post1
for example. Maybe just call it suffix
(assuming that isn't used for something else), since it could consist of multiple PEP 440 segments?
Preserve any non-digit text at the end of the version string, since this frequently indicates a pre-release version like '1.0a1' or
'1.0rc3'. This is the bare minimum to make pre-release versions work.
Yes, we probably could use the
semver
package or even switch tosetuptools_scm
:-D