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

refactor(install): Move value parsing to clap #12547

Merged
merged 2 commits into from
Aug 24, 2023
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Aug 23, 2023

What does this PR try to resolve?

Originally, I thought this was going to help some of my other work but it won't. Instead I decided to finish and post this PR in case there was interest since @weihanglo expressed interest in using Arg::value_parser more and this demonstrates that.

How should we test and review this PR?

A commit at a time. There seemed to be existing tests for some of the error changes.

@rustbot
Copy link
Collaborator

rustbot commented Aug 23, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-install S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow this is really impressive! Much clearer that argparsing should reside in the binary. Thanks for the pull request!

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Aug 24, 2023

📌 Commit 9379325 has been approved by weihanglo

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2023
@bors
Copy link
Contributor

bors commented Aug 24, 2023

⌛ Testing commit 9379325 with merge 4b9a10d...

bors added a commit that referenced this pull request Aug 24, 2023
refactor(install): Move value parsing to clap

### What does this PR try to resolve?

Originally, I thought this was going to help some of my other work but it won't.  Instead I decided to finish and post this PR in case there was interest since `@weihanglo` expressed interest in using `Arg::value_parser` more and this demonstrates that.

### How should we test and review this PR?

A commit at a time.  There seemed to be existing tests for some of the error changes.
@bors
Copy link
Contributor

bors commented Aug 24, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 24, 2023
@weihanglo
Copy link
Member

Resolver proptest issue again #6258.

 failures:
    prop_direct_minimum_version_error_implications
    prop_minimum_version_errors_the_same
    prop_passes_validation
    prop_removing_a_dep_cant_break

We recently hit this as well in #12539.

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2023
@bors
Copy link
Contributor

bors commented Aug 24, 2023

⌛ Testing commit 9379325 with merge 3581425...

@bors
Copy link
Contributor

bors commented Aug 24, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 3581425 to master...

@bors bors merged commit 3581425 into rust-lang:master Aug 24, 2023
18 checks passed
@epage epage deleted the install branch August 24, 2023 17:45
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 26, 2023
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
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area: Command-line interface, option parsing, etc. Command-install S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants