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

Merge semver and semver-parser #125

Closed
lambda-fairy opened this issue Sep 19, 2017 · 9 comments
Closed

Merge semver and semver-parser #125

lambda-fairy opened this issue Sep 19, 2017 · 9 comments

Comments

@lambda-fairy
Copy link

The parsers for versions and version ranges are currently in a separate crate, called semver-parser. This crate also redefines all the data structures, such that the main semver crate has to recurse through the parsed result and convert everything into the correct type.

Is there a reason for this separation? semver does have an extra serde dependency, but we can hide that behind a feature flag if users don't want it. I don't see any other use case for keeping the crates separate, and merging the two crates will avoid this duplicate work.

@KodrAus
Copy link

KodrAus commented Sep 19, 2017

The serde feature is also already optional in semver.

@steveklabnik
Copy link
Contributor

semver-parser has its own dependents too: https://crates.io/crates/semver-parser/reverse_dependencies

One could maybe argue that Version should move into semver-parser, and that semver would re-export it, but basically, any time you want to parse semver versions, but you don't care about things like ranges, only semver-parser is needed.

Basically, I like small, focused crates.

@KodrAus
Copy link

KodrAus commented Oct 1, 2017

This is also discussed in #59

@KodrAus
Copy link

KodrAus commented Oct 10, 2017

I think because semver_parser hasn't received the same doc treatment as semver it feels like an implementation detail rather than a crate in its own right. And an obvious thing to want to do with implementation details is hide them.

So I think if we beef the docs out then semver_parser will stand on its own better.

@udoprog
Copy link
Contributor

udoprog commented Dec 8, 2017

@steveklabnik I'd also like to see this happen. The two crates are currently tightly coupled so that releases must be carefully coordinated (e.g. blocks things like #151 from happening between two releases).

I've spent time working through the existing dependents.

version-sync (renamed duplicate of check-versions)

Uses semver-parser to parse a VersionReq, then implements the compatibility check manually:

https://github.com/mgeisler/version-sync/blob/master/src/lib.rs#L119

Naively it looks like this use-case could be much better served using this crate.
By providing a custom matches implementation it effectively undermines the behavior of this crate.

libvpx-native-sys

Doesn't appear to be using semver-parser any more?

debcargo

Primarily used to parse the VersionReq, then inspect the structure of it.

Used here: https://anonscm.debian.org/git/pkg-rust/debcargo.git/tree/src/debian/dependency.rs

It's worth noting that this uses semver-parser because the structure of VersionReq is private in this crate, but not in semver-parser.
Making the predicates public, or adding accessor methods, would permit them to move.

cargo-tally

Also seems to be a case of providing access to internal structures.

Conclusion

semver-parser could be merged into semver if semver provided access to currently private parts of Version, VersionReq.

@udoprog
Copy link
Contributor

udoprog commented Jan 5, 2018

So I've been needing some of the new semver support (especially #151) for a while, which has blocked me from publishing new versions of my application to crates.io. The slow moving progress on this has caused me to incorporate semver as an internal library (called reproto-semver) which includes all the changes I need.

This effectively unblocks me, and I'm happy to wait until a new version of semver becomes available which does what I need. But I'd like to highlight that I took the approach of merging the parser. This caused a significant reduction in the amount of boilerplate used to communicate between the two crates. The translation was also very straight forward since there's almost a 1:1 mapping between the two.

If you'd like some inspiration of where this project could go if you choose to take this advice of merging the crates, please check out: https://github.com/reproto/reproto/tree/master/lib/semver

@steveklabnik
Copy link
Contributor

Sorry about that! I'd been focusing on semver-parser, but then the holidays happened; I hope to have 1.0 of both libs out by the end of the month.

@udoprog
Copy link
Contributor

udoprog commented Jan 6, 2018

Hey no worries! Take the time you need and I'm here to help if you want it :).

@dtolnay
Copy link
Owner

dtolnay commented May 25, 2021

This has happened in semver 1.0.0.

@dtolnay dtolnay closed this as completed May 25, 2021
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

5 participants