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
SemVer 2.0 and npm semver support #2596
Conversation
| array('>2.0 <=3.0'), | ||
| array('>2.0, <=3.0'), | ||
| array('>2.0 ,<=3.0'), | ||
| array('>2.0 , <=3.0'), |
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.
would it work with a space between <= and 3.0 ?
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.
Yeah it's covered above, but I'll still add a few more cases just to
bulletproof it.
|
Let's do the whole https://npmjs.org/doc/misc/semver.html#Ranges syntax though and not just one part of it I'd say. Can help with it later this week. |
|
Yeah, but this is a first step and looks fine to me to merge :) |
|
I disagree, @naderman. Let's do a full implementation with enough tests to make sure that there are no conflicts with existing syntax. There is little point in ending up with only a half baked implementation in case of incompatibilities, which will give no benefit for the future, but instead just more code to maintain :) |
|
Just pushed complete support and updated the PR description. I'd appreciate if someone can sanity-check the new tests before I merge this. |
|
Looks perfect to me. |
|
Alright then merging and let's hope I didn't miss anything :) |
SemVer 2.0 and npm semver support
|
🚀 |
| @@ -178,10 +183,10 @@ public function normalizeBranch($name) | |||
| return $this->normalize($name); | |||
| } | |||
|
|
|||
| if (preg_match('#^v?(\d+)(\.(?:\d+|[x*]))?(\.(?:\d+|[x*]))?(\.(?:\d+|[x*]))?$#i', $name, $matches)) { | |||
| if (preg_match('#^v?(\d+)(\.(?:\d+|[xX*]))?(\.(?:\d+|[xX*]))?(\.(?:\d+|[xX*]))?$#i', $name, $matches)) { | |||
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.
This change seems unnecessary as you already have the insensitive flag.
Same goes for below, except 370.
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.
Yes you're right I just applied it to all the x's
This brings composer in line with npm's semver package + follows some more semver rules.
^operator for stricter semver ranges1.0 - 2.0ranges+build.metadatasuffixes||is now equivalent to|for OR