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

Version range sets #60

Merged
merged 2 commits into from
Nov 18, 2015
Merged

Version range sets #60

merged 2 commits into from
Nov 18, 2015

Conversation

dirk
Copy link
Contributor

@dirk dirk commented Nov 16, 2015

This is an implementation of PR #57 (tracked in PR #58).

  • Renames VersionReq to VersionSet.
  • Make a new VersionReq that is composed of a set of VersionSet's.
  • Tests for new VersionReq struct.

Renames `VersionReq` to `VersionSet`; make a new `VersionReq` that
is composed of a set of `VersionSet`'s.
steveklabnik added a commit that referenced this pull request Nov 18, 2015
@steveklabnik steveklabnik merged commit 14df18c into dtolnay:master Nov 18, 2015
@steveklabnik
Copy link
Contributor

This looks great, thanks!

@steveklabnik
Copy link
Contributor

@dirk , I feel bad about this, but I've reverted these changes in #71

I want to support this feature, and I don't think this patch is bad, but it introduced regressions into Cargo's test suite, and so for the moment, I'm going to take it out.

@dirk
Copy link
Contributor Author

dirk commented Dec 4, 2015

@steveklabnik: Totally understand! Want to start a discussion in that PR or this one about how to get these changes into a shape that they can be merged? How would you feel about bundling this up with some other improvements and calling it a major version bump?

@steveklabnik
Copy link
Contributor

So, the big thing is that I'd like to move all parsing to nom. I've done that with Version, the question is how to do that with VersionReq. At the moment, I'm trying to debug #54 , and why it also caused failures, but I'd like to do that and re-build this functionality on top of it, if that makes sense?

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

Successfully merging this pull request may close these issues.

None yet

2 participants