fix(version): Versions should support int64 integers #35
Conversation
| @@ -32,6 +32,7 @@ func TestNewVersion(t *testing.T) { | |||
| {"v1.2.3-rc1-with-hypen", false}, | |||
| {"1.2.3.4", true}, | |||
| {"v1.2.3.4", true}, | |||
| {"20161202202307-sha.e8fc5e5", false}, | |||
There was a problem hiding this comment.
The test you're using isn't really semver. What about using the example v2.3.5-20161202202307-sha.e8fc5e5 from the ticket?
On the tip of master the test for v2.3.5-20161202202307-sha.e8fc5e5 passes for this test. There should be a constraints test for this which fails for this PR.
It's the constraint handling rather than the version handling that's failing for this case.
| @@ -59,14 +59,14 @@ func NewVersion(v string) (*Version, error) { | |||
| } | |||
|
|
|||
| var temp int64 | |||
| temp, err := strconv.ParseInt(m[1], 10, 32) | |||
| temp, err := strconv.ParseInt(m[1], 10, 64) | |||
There was a problem hiding this comment.
I'm ok with the change to 64 even though it's not impacting.
|
Yes...when i call NewConstraint(">=v2.3.5-20161202202307-sha.e8fc5e5") , it is calling parseConstraint(">= v2.3.5") and parseConstraint("<= 20161202202307-sha.e8fc5e5") which then calls NewVersion() (https://github.com/Masterminds/semver/blob/master/constraints.go#L185) which is where it is failing with error |
|
Interesting...that's a bug in the parser then; it's treating that first dash as a range operator between two separate versions, rather than conjoining the two parts of the string. |
|
I believe the underlying issue is here. `\s*(%s)\s*-\s*(%s)\s*`,Should be `\s*(%s)\s+-\s+(%s)\s*`,There should be one or more spaces. This is a typo. The intent was the required space in the hyphen range syntax. |
|
@mattfarina i made the change for the regex.Also i kept the change for the int64 which i can remove if you want. |
fixes #34