Skip to content
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

Allow any prefix to semver tag #3

Merged
merged 1 commit into from Dec 21, 2019
Merged

Allow any prefix to semver tag #3

merged 1 commit into from Dec 21, 2019

Conversation

tajobe
Copy link
Contributor

@tajobe tajobe commented Dec 18, 2019

We have a use case for allowing prefixes other than v on an otherwise semver tag. This allows any (or no) prefix tag to work, as long as it ends in a semantic version.

I don't see any contributing guidelines so I didn't want to make an assumption about updating the version, but let me know if I should add a [1.1.0] as opposed to [Unreleased] (and update the docker label).

@anton-yurchenko
Copy link
Owner

Thanks @tajobe for your contribution!

Allowing any tag seems to be conflicting with the Parse Tag to match Semantic Versioning feature.

Never the less, allowing non v prefixed tags are indeed a good enhancement. It was already discussed and approved on semver PR-1.

Regarding your use-case, we might disable semver check altogether and match any tag. It should probably be done via another env.var ENFORCE_SEMVER=false.

Anyway, as this action gains popularity, I decided to spend some time refactoring the code first.
It is prefered to not introduce changes on previous non-refactored codebase. Unfortunately, because of that reason, your PR cannot be merged.

I will keep this PR open and update here whenever proposed change is released or requires assistance.

P.S: contributing guidelines will be added in the coming days too

@anton-yurchenko anton-yurchenko self-assigned this Dec 20, 2019
@tajobe
Copy link
Contributor Author

tajobe commented Dec 21, 2019

Hi @anton-yurchenko . I'm a bit confused as a prefix to a semver tag is not prescribed in semver 2.0 at all. In fact, from the FAQ, v1.2.3 is also not a semantic version, but rather the version is still 1.2.3 and v is simply a (common) prefix like any other.

Is “v1.2.3” a semantic version?
No, “v1.2.3” is not a semantic version. However, prefixing a semantic version with a “v” is a common way (in English) to indicate it is a version number. Abbreviating “version” as “v” is often seen with version control. Example: git tag v1.2.3 -m "Release version 1.2.3", in which case “v1.2.3” is a tag name and the semantic version is “1.2.3”.

This change does not allow any tag, rather any prefix and still expects to parse a semantic version, which preserves the Parse Tag to match Semantic Versioning feature. I can see how it would be preferable to make any prefix configurable, but I still desire to have the version parsed, not thrown out/disabled. Happy to help with that if I can.

I can see a desire to not actively work on code being refactored though I should also mention there is a bug with the PRE_RELEASE var overwriting the Draft configuration. I assume this shouldn't wait for a refactor as drafts are broken. I can back out the semver change and PR that as well. See https://github.com/SimpleMC/git-release/commit/4f15752384c2416248d16d651d5651e5cae7d4a7

Edit: Thanks for getting back to me about the PR, always appreciate when a maintainer takes the time to explain a position even if I don't necessarily agree.

@tajobe
Copy link
Contributor Author

tajobe commented Dec 21, 2019

Also, as an example of this being common, allegro/axion-release-plugin for Gradle defaults their git tag prefix to release-, but parses the version without it.

@anton-yurchenko
Copy link
Owner

oh right @tajobe! Sorry for confusing you, I got it wrong in the first place.
Gladly merged, thank you.

P.S: draft bug will be fixed as a part of this release. Refactor is underway anyway as it addresses other possible issues, testing, ...

@anton-yurchenko anton-yurchenko merged commit 9c23ec9 into anton-yurchenko:master Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants