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

DynVer allows Invalid but Matching Tags #92

Open
apeschel opened this issue Mar 19, 2019 · 3 comments
Open

DynVer allows Invalid but Matching Tags #92

apeschel opened this issue Mar 19, 2019 · 3 comments

Comments

@apeschel
Copy link

apeschel commented Mar 19, 2019

It seems like dynver is trying to produce a valid SemVer version as its version, but at the moment, the tag parser is a bit too flexible in what it allows:

Non SemVer tags

Input:
v123456
Expected Result:
Error or ignore tag
Actual Result:
version = 123456

Input:
v1.2.3.4
Expected Result:
Error or ignore tag
Actual Result:
version = 1.2.3.4

Input:
v1_this_really_shouldn't_be_a_valid_dynver_tag_@&%
Expected Result:
Error or ignore tag
Actual Result:
version = 1_this_really_shouldn't_be_a_valid_dynver_tag_@&%

@apeschel apeschel changed the title Invalid but Matching Tags Produce Invalid SemVer Version or Break DynVer DynVer allows Invalid but Matching Tags Mar 19, 2019
@dwijnand
Copy link
Member

dwijnand commented Mar 19, 2019

Thank you for the issue report! You've given me food for thought about sbt-dynver's implementation. 😄

So it's not an explicit, stated goal for sbt-dynver to be strictly SemVer compliant, and as such I have no problem with it matching "v123456" and "v1.2.3.4".

I also don't feel strongly that "v1_this_really_shouldn't_be_a_valid_dynver_tag_@&%" should be an invalid version string. It's weird, but I think a version one might use is something like "1.2.3_scalaz_7.3" (specs2 used to do something like this at one point). So where do we draw the line and how?

Is the current implementation causing a problem for you? I'm wondering what motivated the issue and where your interests lie. Do you have an opinion on what you think should be different in sbt-dynver?

@apeschel
Copy link
Author

apeschel commented Mar 19, 2019

I can see how on one hand how allowing the flexible matching can be see as a benefit, since it means the plugin can be used for project that aren't following a fully SemVer compliant versioning system.

On the flip side, it can cause problems for teams using SemVer compliant versioning schemes, since anyone with commit access can potentially cause chaos with the build system by pushing a non-SemVer tag upstream, which will result in non-SemVer builds that could potentially gum up the build system.

Maybe a compromise could be to have a setting to ignore non-SemVer tags?

@dwijnand
Copy link
Member

Ah, I see. I think that's a worthy case to try and work against. But rather than introduce more complexity (with alternative validations/parsing) I think one could use the https://github.com/dwijnand/sbt-dynver#sanity-checking-the-version to fast-fail a build load if non-complying version tags are pushed. That way teams are in control of deciding exactly which version scheme to restrict towards (strict SemVer or some variant of SemVer of their choosing). I could detail this fact in that section. Do you think that's reasonable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants