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.

Sign up for GitHub

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

Jump to bottom

fix(update): Tweak CLI behavior #12428

Merged
merged 3 commits into from
Aug 1, 2023
Merged

fix(update): Tweak CLI behavior #12428

merged 3 commits into from
Aug 1, 2023

Conversation

epage
Copy link
Contributor

@epage epage commented Aug 1, 2023

What does this PR try to resolve?

When looking at cargo update for #12425, I noticed that the two flags related to --package were not next to it or each other. I figured grouping them like that would make things easier to browse.

When looking into that, I noticed that the two flags conflict and figured we'd provide a better error message if we did that through clap.

How should we test and review this PR?

Looking per commit will help show the behavior changes.

Additional information

I wanted to scope this to being simple, non-controversial, low effort, incremental improvements with this change so I did not look into the history of --aggressive not requiring --package like --precise does and figure out if there is any consistency we can be working towards.

@rustbot
Copy link
Collaborator

rustbot commented Aug 1, 2023

r? @ehuss

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

@rustbot rustbot added A-cli Area: Command-line interface, option parsing, etc. Command-update S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 1, 2023
weihanglo
weihanglo reviewed Aug 1, 2023
View reviewed changes
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.

Should this be removed as well?

cargo/src/cargo/ops/cargo_generate_lockfile.rs

Lines 39 to 41 in c91a693

if opts.aggressive && opts.precise.is_some() {
anyhow::bail!("cannot specify both aggressive and precise simultaneously")
}

BTW, it's great to see help text is tracked :)

@epage
Copy link
Contributor Author

epage commented Aug 1, 2023

Should this be removed as well?

cargo/src/cargo/ops/cargo_generate_lockfile.rs

Lines 39 to 41 in c91a693

if opts.aggressive && opts.precise.is_some() {
anyhow::bail!("cannot specify both aggressive and precise simultaneously")
}

I left that because that check is likely still meaningful from the perspective of that API

@weihanglo
Copy link
Member

Make sense. Thank you!

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 1, 2023

📌 Commit 4fd0c3c 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 1, 2023
@bors
Copy link
Collaborator

bors commented Aug 1, 2023

⌛ Testing commit 4fd0c3c with merge baebd2b...

@bors
Copy link
Collaborator

bors commented Aug 1, 2023

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

@bors bors merged commit baebd2b into rust-lang:master Aug 1, 2023
18 checks passed
@epage epage deleted the update branch August 1, 2023 16:31
@weihanglo weihanglo mentioned this pull request Aug 2, 2023
Merged
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Aug 2, 2023
@Noratrieb
Rollup merge of rust-lang#114364 - weihanglo:update-cargo, r=weihanglo
9c7d67f 
Update cargo

8 commits in c91a693e7977e33a1064b63a5daf5fb689f01651..6dc1deaddf62c7748c9097c7ea88e9ec77ff1a1a
2023-07-31 00:26:46 +0000 to 2023-08-02 00:23:54 +0000
- `#[allow(internal_features)]` in RUSTC_BOOTSTRAP test (rust-lang/cargo#12429)
- ci: rewrite bump check and respect semver (rust-lang/cargo#12395)
- fix(update): Tweak CLI behavior (rust-lang/cargo#12428)
- chore(deps): update compatible (rust-lang/cargo#12426)
- Display crate version on timings graph (rust-lang/cargo#12420)
- chore(deps): update alpine docker tag to v3.18 (rust-lang/cargo#12427)
- Use thiserror for credential provider errors (rust-lang/cargo#12424)
- Clarify in `--help` that `cargo test --all-targets` excludes doctests (rust-lang/cargo#12422)

r? `@ghost`
Noratrieb added a commit to Noratrieb/rust that referenced this pull request Aug 2, 2023
@Noratrieb
Rollup merge of rust-lang#114364 - weihanglo:update-cargo, r=weihanglo
a771548 
Update cargo

8 commits in c91a693e7977e33a1064b63a5daf5fb689f01651..6dc1deaddf62c7748c9097c7ea88e9ec77ff1a1a
2023-07-31 00:26:46 +0000 to 2023-08-02 00:23:54 +0000
- `#[allow(internal_features)]` in RUSTC_BOOTSTRAP test (rust-lang/cargo#12429)
- ci: rewrite bump check and respect semver (rust-lang/cargo#12395)
- fix(update): Tweak CLI behavior (rust-lang/cargo#12428)
- chore(deps): update compatible (rust-lang/cargo#12426)
- Display crate version on timings graph (rust-lang/cargo#12420)
- chore(deps): update alpine docker tag to v3.18 (rust-lang/cargo#12427)
- Use thiserror for credential provider errors (rust-lang/cargo#12424)
- Clarify in `--help` that `cargo test --all-targets` excludes doctests (rust-lang/cargo#12422)

r? ``@ghost``
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 2, 2023
@bors
Auto merge of rust-lang#114364 - weihanglo:update-cargo, r=weihanglo
8131b97 
Update cargo

10 commits in c91a693e7977e33a1064b63a5daf5fb689f01651..020651c52257052d28f6fd83fbecf5cfa1ed516c
2023-07-31 00:26:46 +0000 to 2023-08-02 16:00:37 +0000
- Update rustix to 0.38.6 (rust-lang/cargo#12436)
- replace `master` branch by default branch in documentation (rust-lang/cargo#12435)
- `#[allow(internal_features)]` in RUSTC_BOOTSTRAP test (rust-lang/cargo#12429)
- ci: rewrite bump check and respect semver (rust-lang/cargo#12395)
- fix(update): Tweak CLI behavior (rust-lang/cargo#12428)
- chore(deps): update compatible (rust-lang/cargo#12426)
- Display crate version on timings graph (rust-lang/cargo#12420)
- chore(deps): update alpine docker tag to v3.18 (rust-lang/cargo#12427)
- Use thiserror for credential provider errors (rust-lang/cargo#12424)
- Clarify in `--help` that `cargo test --all-targets` excludes doctests (rust-lang/cargo#12422)

r? `@ghost`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Aug 3, 2023
@bors
Auto merge of #114364 - weihanglo:update-cargo, r=weihanglo
eb72caa 
Update cargo

10 commits in c91a693e7977e33a1064b63a5daf5fb689f01651..020651c52257052d28f6fd83fbecf5cfa1ed516c
2023-07-31 00:26:46 +0000 to 2023-08-02 16:00:37 +0000
- Update rustix to 0.38.6 (rust-lang/cargo#12436)
- replace `master` branch by default branch in documentation (rust-lang/cargo#12435)
- `#[allow(internal_features)]` in RUSTC_BOOTSTRAP test (rust-lang/cargo#12429)
- ci: rewrite bump check and respect semver (rust-lang/cargo#12395)
- fix(update): Tweak CLI behavior (rust-lang/cargo#12428)
- chore(deps): update compatible (rust-lang/cargo#12426)
- Display crate version on timings graph (rust-lang/cargo#12420)
- chore(deps): update alpine docker tag to v3.18 (rust-lang/cargo#12427)
- Use thiserror for credential provider errors (rust-lang/cargo#12424)
- Clarify in `--help` that `cargo test --all-targets` excludes doctests (rust-lang/cargo#12422)

r? `@ghost`
@ehuss ehuss added this to the 1.73.0 milestone Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@ehuss ehuss

Labels
A-cli Area: Command-line interface, option parsing, etc. Command-update S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.73.0
Development

Successfully merging this pull request may close these issues.
5 participants
@epage @rustbot @weihanglo @bors @ehuss