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

Fix semantic version #9868

Merged
merged 2 commits into from
Nov 6, 2020
Merged

Conversation

hugopl
Copy link
Contributor

@hugopl hugopl commented Oct 31, 2020

The regexp used in the SemanticVersion class were accepting bad versions like 01.2.3 and 1.2.3-hey. and rejecting good versions like 1.2.3+hey.ho.

I just picked the regexp provided by https://semver.org/ itself and added few tests.

Changes:
 - Accept valid semantic versions like "1.2.3+foo.bar".
 - Reject invalid semantic versions like "1.2.3-foo." and a lot more that the parse were accepting.
@bcardiff bcardiff added this to the 1.0.0 milestone Nov 4, 2020
@bcardiff
Copy link
Member

bcardiff commented Nov 4, 2020

@jhass is this really a breaking-change for you? It is a more strict validation, but within the original intended behavior.

@jhass
Copy link
Member

jhass commented Nov 4, 2020

It has the potential to break existing projects using versions that don't parse by the more strict validation now, so yes, technically it is.

It's a good change, I just felt the label is there to document such little gotchas.

@bcardiff
Copy link
Member

bcardiff commented Nov 4, 2020

Ok 👍

@bcardiff bcardiff merged commit 0a9f13e into crystal-lang:master Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants