-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
refactor: Pull out cargo-add MSRV code for reuse #12553
Conversation
By using the `PartialVersion` type with serde, we get the context for the error automatically.
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
Since we have tests for a couple of cases, I figured we could improve the error messages for them.
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.
Looks good. The review comments aren't really important. Feel free to mark as resolved and I'll come back and merge.
36cf9cd
to
1701b4e
Compare
Thanks! @bors r+ |
☀️ Test successful - checks-actions |
Update cargo 13 commits in 2cc50bc0b63ad20da193e002ba11d391af0104b7..925280f028db3a322935e040719a0754703947cf 2023-08-22 22:43:08 +0000 to 2023-08-25 21:16:44 +0000 - string leek is stable (rust-lang/cargo#12559) - refactor: Pull out cargo-add MSRV code for reuse (rust-lang/cargo#12553) - Contrib: Add process for security responses. (rust-lang/cargo#12487) - Support dependencies from registries for artifact dependencies, take 2 (rust-lang/cargo#12421) - fix(toml): Improve parse errors (rust-lang/cargo#12556) - Create dedicated unstable flag for asymmetric-token (rust-lang/cargo#12551) - chore(deps): update latest msrv to v1.72.0 (rust-lang/cargo#12549) - changelog: add link to CVE-2023-40030 (rust-lang/cargo#12550) - refactor(install): Move value parsing to clap (rust-lang/cargo#12547) - fix: Set MSRV for internal packages (rust-lang/cargo#12381) - doc: fix two links to tracing docs (rust-lang/cargo#12537) - use AND search when having multiple terms (rust-lang/cargo#12548) - fix(log): Use a more compact relative-time format (rust-lang/cargo#12542) r? ghost
What does this PR try to resolve?
#12078 added MSRV code in
cargo add
. Our assumption when writing it is that we'd need to generalize the code before reusing it in other places, likecargo install
. This PR focused purely on that refactor because I'm hopeful it will be useful for other work I'm doing. Despite not having a user for this yet, I think thecargo install
case is inevitable and I feel this does a bit to clean up MSRV related code by using a more specific type everywhere.How should we test and review this PR?
Each commit gradually progresses things along