-
-
Notifications
You must be signed in to change notification settings - Fork 132
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-parser 0.10.0 #209
semver-parser 0.10.0 #209
Conversation
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 is amazing, and we should . Thanks so much again for doing this.
I have one quick question, since you've been in this code lately, but IMHO we should cut the semver-parser release, update this to use it, and cut a new semver release, probably over the next few days.
tests/deprecation.rs
Outdated
@@ -10,7 +10,8 @@ fn test_regressions() { | |||
("0.1.0.", VersionReq::parse("0.1.0").unwrap()), | |||
("0.3.1.3", VersionReq::parse("0.3.13").unwrap()), | |||
("0.2*", VersionReq::parse("0.2.*").unwrap()), | |||
("*.0", VersionReq::any()), | |||
// TODO: this actually parses as '*' now, not sure if that's OK | |||
// ("*.0", VersionReq::any()), |
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.
So, I am actually not sure what the difference here is between "parses as *" and Any, that is, they should be the same?
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.
When I was comparing it on https://semver.npmjs.com/, they both resolve to the same set of versions. It's now not "deprecated". The question is if we care about making it deprecated (want to discourage people from using this)?
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.
Ah yes, the deprecation. Thanks :) It is probably fine to un-deprecate it, but I'd like to triple check against semver/semver#584 real quick to see if it's acceptable in that grammar....
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.
There's also some other TODOs we need to either clean up in this PR or another one before we ship a new version of both semver
and semver-parser
that @mikrowstew left behind.
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.
I think semver/semver#584 disallows it, but fwiw, node-semver parses it as *
as well. If y'all move forward with doing it that way, we should update the spec to make that official.
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.
@isaacs that sounds good to me.
Wow, this is great! Thank you so much for seeing this through to the end ❤️ |
@hone checking on this; did you want me to merge it now, or did you want to try to tackle the TODOs first? It wasn't clear to me :) |
@steveklabnik we can merge this in now and I can just do the other TODOs in a separate PR. We would just want to address them before releasing either crate. |
@mikrostew thanks for doing all the hard legwork here. :) |
See dtolnay#58 for more info.
See discussion here: dtolnay#209 (comment)
@steveklabnik thoughts on renaming |
Yeah it should probably be |
Technically these are associated with the tools, not the languages. dtolnay/semver#209 (comment)
Technically these are associated with the tools, not the languages. dtolnay#209 (comment)
(Going to push a commit to un-patch semver-parser after this. Thanks for all the work, @hone <3) |
This combines the changes from mikrostew/semver@new-parser and the current master branch which makes it compatible with
semver-parser
0.10.0
that hasn't quite been released. In order for the tests pass, it patches thesemver-parser
dependency to use GitHub.Tracking Items Left:
=
works with build metadata #88VersionReq::any()
as'*'
VersionReq::parse_compat
doc exampleCompat::Node
toCompat::Npm
0.10.0
ofsemver-parser
cratesemver-parser
inCargo.toml