Out-of-band crate evaluation for 2017-10-13: semver

libs
1

Out-of-band crate evaluation for 2017-10-13: semver

For additional contribution opportunities, see the main libz blitz thread.

This post is a wiki. Feel free to edit it.

Links

semver

semver-parser

Needs your help!

Anything that is not checked off still needs your help! There is no need to sign up or ask for permission - follow any of these links and leave your thoughts:

Guidelines checklist

Legend

  • [y] = guideline is adhered to, no work needed.
  • [n] = guideline may need work, see comments nearby
  • [/] = guideline not applicable to this crate

Checklist

Guidelines checklist

This document is collaboratively editable. Pick a few of the guidelines, compare the semver crate against them, and fill in the checklist with [y] if the crate conforms to the guideline, [n] if the crate does not conform, and [/] if the guideline does not apply to this crate.

For more details, see

  - [n] Crate name is a palindrome (C-PALINDROME)
   - my_crate backwards is etarc_ym which is not the same as my_crate

Cookbook recipes

Cookbook example ideas

Come up with ideas for nice introductory examples of using semver, possibly in combination with other crates, that would be good to show in the Rust Cookbook. Please leave a comment in that issue with your ideas! You don't necessarily have to write the example code yourself but PRs are always welcome.

API guideline updates

What lessons can we learn from semver that will be broadly applicable to other crates? Please leave a comment in that issue with your ideas!

Discussion topics

Anything that's not a concrete crate issue yet. We want to eventually promote any topics here into actionable crate issues below.

Version docs

The docs for Version could be improved. What specific things could make them easier to consume?

Range syntax

Does the crate intend to implement the remaining parts of the NPM semver range syntax, like the || operator, whitespace-separated (rather than comma-separated) range sets, and hyphen ranges?

This could be implemented by adding a layer above VersionReq to semver-parser called something like RangeSet that contains a list of VersionReqs. In semver we can change the internals to align with semver_parser::RangeSet instead of semver_parser::VersionReq.

Naming

Should we rename some of the types to match those in the grammar?

  • Predicate -> Comparator
  • VersionReq -> ComparatorSet or Range

Non-exhaustive semver-parser

Types in semver-parser are currently composed of public fields. This makes the set of potentially breaking changes larger. If this isn't desirable then we should make the types non-exhaustive, this can be a private _non_exhaustive: () field.

Crate issues

Issues to file against the semver crate.

semver_parser issues to file

  • Implement Clone, PartialEq, Eq, PartialOrd, Ord, Hash for VersionReq
  • Implement Clone, PartialOrd, Ord, Hash for Version
  • Add examples to items and methods in semver_parser. Focus on parsing
  • Add readme, keywords and categories to Cargo.toml
  • Add html_root_url attribute to crate root
  • Make structures with public fields non-exhaustive
  • Rename Predicate to Comparator
  • Rename VersionReq to Range

Bigger items

These are some bigger things that would need to be broken down more to be friendly for contributors. We might want to do them first.

Implementing range sets
  • Add a RangeSet type that can parse a collection of ranges between ||s
Docs
  • Document the semver_parser crate:
    • What is this crate about?
    • How is it different to semver?
    • Exactly what does it parse?

semver issues to file

  • Use ? instead of unwrap in examples
  • Document error cases on Version::parse
  • Document error cases on VersionReq::parse
  • Turn references to other crate items in prose into links
  • Add keywords and categories to Cargo.toml
  • Fix link to docs.rs for documentation in Cargo.toml
  • Add html_root_url attribute to crate root
  • Make error types non-exhaustive (or hide variants in a struct using private fields)

Bigger items

These are some bigger things that would need to be broken down more to be friendly for contributors. We might want to do them first.

Hiding structure definitions
  • Version uses public fields. We should make an effort to hide these and the exact kind of collection fields are stored in
Implementing range sets
  • Support the || operator on sets of Ranges by making VersionReq use a range set internally instead of a single range

How are we tracking?

Pre-review checklist

  • Create evaluation thread based on this template
  • Work with author and issue tracker to identify known blockers
  • Compare crate to guidelines by filling in checklist
  • Record other questions and notes about crate
  • Draft several use case statements to serve as cookbook examples
  • Record recommendations for updated guidelines

Post-review checklist

  • Create new issues and tracking issue on crate's issue tracker
  • Solicit use cases for cookbook examples related to the crate
  • File issues to implement cookbook examples
  • File issues to update guidelines
  • Post all approachable issues to TWiR call for participation thread
  • Update links in coordination thread
3 Likes
2

Let’s include the semver-parser in this evaluation too! Its API is a subset of semver, so most of the same feedback applies.

3

@mbrubeck raised an interesting question about whether semver intends to support the rest of the NPM semver range syntax . I think there are a few points that come out of this:

  • Should semver aim for feature parity with NPM semver?
  • Are there any of these features that could only be implemented in a breaking way? (I’m thinking not, but am not sure if there are any semantic differences in range comparison yet)
  • Are there any of these features that are non-breaking that we would still want before a stable release? I’m feeling like there’s no reason to punt features we want in semver until after a 1.0 since there isn’t a hard timeline for it anyways

Another question: semver is used by cargo and docs.rs, right? So if we support more predicates in semver does that mean that cargo will automagically support them when it upgrades?

Does anyone have any other thoughts?

1 Like
4

I think it's useful to match the npm range syntax, mostly because it has a well-designed and well-documented spec. (For example, I often link to the npm docs on prerelease tags to explain the rationale for Cargo’s behavior, which matches npm’s.)

Are there any of these features that could only be implemented in a breaking way? (I’m thinking not, but am not sure if there are any semantic differences in range comparison yet)

I agree, I think they could be added backward-compatibly.

Adding support for the || operator would require a new representation of semver ranges. Currently a rage is parsed into a VersionReq, which represents what the spec calls a “comparator set”. The VersionReq contains a list of Predicates, which are what the spec calls “comparators”:

pub struct VersionReq {
    pub predicates: Vec<Predicate>,
}

But with the addition of the || tag, a range should be parsed into a list of comparator sets. This could be done backward-compatibly by adding a new type to semver-parser:

pub struct RangeSet {
    pub ranges: Vec<VersionReq>,
}

The semver crate has an identical VersionReq type except that its predicates field is private. That crate could either add a new RangeSet type too, or it could change the internals of its VersionReq to hold a list of comparator sets, instead of a single comparator set. The latter option has the advantage that it would not require downstream crates to change anything in order to start using the || operator, though it has the disadvantage that the naming would then be out of sync with the semver-parser crate.

Are there any of these features that are non-breaking that we would still want before a stable release?

If nothing else, it might be worth changing some public type names to match the terms used in the npm spec or grammar, just for clarity:

  • Predicate -> Comparator
  • VersionReq -> ComparatorSet or Range
  • new type: RangeSet

(If we plan to change semver::VersionReq to represent a RangeSet in the future, then it should be renamed to RangeSet now instead of ComparatorSet or Range.)

1 Like
5

In general, my desire is feature-parity to npm, personally.

6

A minor detail perhaps, but since semver is currently uncategorized, I think the Configuration category would be a good fit.

7

Yeah, I haven’t looked closely at categories yet, this is a good thing to think about! Configuration seems… okay, looking at the other crates in it. It’s not exactly config related, but, idk.

Reading over the categories, I bet I could make it no_std, which would fit into that category. semver-parser would belong in Parser Implementations, and arguably so does semver, even though it uses semver-parser to do so.

Other than that, I’m not sure where it fits in. Hm.

8

Any thoughts on deriving Default for version? Its not super important but I was quite surprising when I found out its not there.

9

Hmm, I'm not sure what a default version should be. 1.0.0? 0.0.1? Not having Default seems ok to me.

10

I don't have a lot of experience with no_std support, would we need to build no_std alternatives to Vec and String? Is there a std of no_std that's got some fundamental things like that?

11

Agreed. I was thinking about 0.0.1 but you're right about the almost certain confusion. Lets leave it as is.

12

I’ve had a poke around the semver::VersionReq and semver_parser:range::VersionReq types and I’m thinking keeping the crates separate is reasonable, and the path @mbrubeck described seems like a good approach to me.

semver-parser's public fields are a bit problematic for backwards compatibility though. With the current design we’d have to make sure semver_parser isn’t a public dependency of semver so it can make breaking changes if it needs to and not affect semver's stability.

1 Like
13

What do you all think about making semver-parser structures non exhaustive? You wouldn’t be able to construct or exhaustively match on them but could add new fields without needing to break.

14

I’ve pushed our end date back a week with RustFest in the middle I don’t think I’ve given this evaluation enough attention yet.

If anyone has any more thoughts on semver and semver-parser please throw them out there!

15

I’ve added a bit more detail to the evaluation checklist in preparation for collecting some issues.

I’m thinking we should try and hide some implementation details on semver::Version a little more. An effort to completely hide the structure definition might look something like:

pub struct Version {
    major: u64,
    minor: u64,
    patch: u64,
    pre: Pre,
    build: Build,
}

pub struct Pre(Vec<Identifier>);
pub struct Build(Vec<Identifier>);

pub struct Iter<'a>(std::slice::Iter<'a, Identifier>);
pub struct IntoIter(std::vec::IntoIter<Identifier>);

impl Version {
    pub fn major(&self) -> u64;
    pub fn minor(&self) -> u64;
    pub fn patch(&self) -> u64;
    pub fn pre(&self) -> &Pre;
    pub fn build(&self) -> &Build;

    pub fn as_parts(&self) -> (u64, u64, u64, &Pre, &Build);
    pub fn into_parts(self) -> (u64, u64, u64, Pre, Build);
}

impl Pre {
    pub fn iter(&self) -> Iter;
}

impl FromIterator<Identifier> for Pre {
    ...
}

impl IntoIterator for Pre {
    type Item = Identifier;
    type IntoIter = IntoIter;

    ...
}

impl<'a> IntoIterator for &'a Pre {
    type Item = &'a Identifier;
    type IntoIter = Iter<'a>;

    ...
}

impl Extend<Identifier> for Pre {
    ...
}

impl Build {
    pub fn iter(&self) -> Iter;
}

impl FromIterator<Identifier> for Build {
    ...
}

impl IntoIterator for Build {
    type Item = Identifier;
    type IntoIter = IntoIter;

    ...
}

impl<'a> IntoIterator for &'a Build {
    type Item = &'a Identifier;
    type IntoIter = Iter<'a>;

    ...
}

impl Extend<Identifier> for Build {
    ...
}

Which is a lot of pseudo-code! Turns out collections require a fair amount of boilerplate for you to make the most of them.

Does anyone have any thoughts on this?

1 Like
16

Sorry, spamming this thread again. I’ve got a list of some issues to file against semver and semver_parser. Is there anything anyone would like to add or thinks shouldn’t be there?

Please feel free to just throw them in to the OP! It is a wiki so anyone can edit it.

17

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.