Skip to content

fix(update): Tweak CLI behavior#12428

Merged
bors merged 3 commits intorust-lang:masterfrom
epage:update
Aug 1, 2023
Merged

fix(update): Tweak CLI behavior#12428
bors merged 3 commits intorust-lang:masterfrom
epage:update

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

weihanglo commented Aug 1, 2023

Make sense. Thank you!

@bors r+

@bors
Copy link
Contributor

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
Contributor

bors commented Aug 1, 2023

⌛ Testing commit 4fd0c3c with merge baebd2b...

@bors
Copy link
Contributor

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
@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