Skip to content

Conversation

cassaundra
Copy link
Contributor

What does this PR try to resolve?

This PR continues the deferred work of #11099 by adding documentation for the cargo remove subcommand.

How should we test and review this PR?

Ensure that the documentation renders correctly and appropriately covers the feature.

@rust-highfive
Copy link

r? @epage

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 12, 2022
@epage epage mentioned this pull request Oct 12, 2022
5 tasks
@epage
Copy link
Contributor

epage commented Oct 13, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Oct 13, 2022

📌 Commit 74b29d2 has been approved by epage

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 Oct 13, 2022
@bors
Copy link
Contributor

bors commented Oct 13, 2022

⌛ Testing commit 74b29d2 with merge 36f0b6a...

bors added a commit that referenced this pull request Oct 13, 2022
Document `cargo remove`

### What does this PR try to resolve?

This PR continues the deferred work of #11099 by adding documentation for the cargo remove subcommand.

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

Ensure that the documentation renders correctly and appropriately covers the feature.
@cassaundra
Copy link
Contributor Author

Not sure what that failing test is about. I suppose I'll just rebase to master and hope that resolves it.

@epage
Copy link
Contributor

epage commented Oct 13, 2022

sigh new clap release changed some things we validate but we don't keep a lockfile for our own purposes (we are part of the rust-lang/rust workspace)

I'll get the tests fixed

@bors
Copy link
Contributor

bors commented Oct 13, 2022

💔 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 Oct 13, 2022
@epage
Copy link
Contributor

epage commented Oct 13, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Oct 13, 2022

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.

@bors
Copy link
Contributor

bors commented Oct 13, 2022

📌 Commit 74b29d2 has been approved by epage

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 Oct 13, 2022
@bors
Copy link
Contributor

bors commented Oct 13, 2022

⌛ Testing commit 74b29d2 with merge 72b4551...

bors added a commit that referenced this pull request Oct 13, 2022
Document `cargo remove`

### What does this PR try to resolve?

This PR continues the deferred work of #11099 by adding documentation for the cargo remove subcommand.

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

Ensure that the documentation renders correctly and appropriately covers the feature.
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

{{#options}}
{{> options-manifest-path }}

{{> options-locked }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Although cargo rm technically accepts --locked/--frozen, I don't think it should be documented here because it prevents cargo rm from working. It is somewhat a bug that --locked/--frozen are global flags. They should have been added on a per-command basis, but that may be difficult to fix now.

This may require separating --offline from options-locked.md so that it can be kept. (See also #11237)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can certainly move --offline from options-locked.md to resolve that. Is that something you'd like in a separate PR?

Ideally I imagine we'd make that change at the same time we make --locked/--frozen not global, so that the problem is fixed and documentation is correctly maintained, but maybe that's not a problem worth solving right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

side note: I think whatever we do, the shell completions should match the docs (see #11204)

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that something you'd like in a separate PR?

Either here or separate is fine with me. Maybe separate might be better if it involves changing a lot of files?

@cassaundra
Copy link
Contributor Author

Can you also make sure that the https://github.com/rust-lang/cargo/blob/master/src/doc/src/commands/manifest-commands.md index is updated?

Done.

@epage
Copy link
Contributor

epage commented Oct 14, 2022

Since it sounds like the only remaining issue is going to be split into a separate PR, I'm going ahead and merging this.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 14, 2022

📌 Commit 9bacebf has been approved by epage

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Oct 14, 2022

⌛ Testing commit 9bacebf with merge 2f93984...

@bors
Copy link
Contributor

bors commented Oct 14, 2022

☀️ Test successful - checks-actions
Approved by: epage
Pushing 2f93984 to master...

@bors bors merged commit 2f93984 into rust-lang:master Oct 14, 2022
weihanglo added a commit to weihanglo/rust that referenced this pull request Oct 18, 2022
6 commits in b332991a57c9d055f1864de1eed93e2178d49440..3ff044334f0567ce1481c78603aeee7211b91623
2022-10-13 22:05:28 +0000 to 2022-10-17 20:25:00 +0000
- Some tiny refactors around `ops::cargo_compile` (rust-lang/cargo#11243)
- Polish docs for module `build_context` (rust-lang/cargo#11241)
- Remove sparse+ prefix for index.crates.io (rust-lang/cargo#11247)
- docs(add): Add missing flags to reference (rust-lang/cargo#11240)
- Document `cargo remove` (rust-lang/cargo#11227)
- fix: Update help headings to  match clap (rust-lang/cargo#11239)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 19, 2022
Update cargo

6 commits in b332991a57c9d055f1864de1eed93e2178d49440..3ff044334f0567ce1481c78603aeee7211b91623 2022-10-13 22:05:28 +0000 to 2022-10-17 20:25:00 +0000
- Some tiny refactors around `ops::cargo_compile` (rust-lang/cargo#11243)
- Polish docs for module `build_context` (rust-lang/cargo#11241)
- Remove sparse+ prefix for index.crates.io (rust-lang/cargo#11247)
- docs(add): Add missing flags to reference (rust-lang/cargo#11240)
- Document `cargo remove` (rust-lang/cargo#11227)
- fix: Update help headings to  match clap (rust-lang/cargo#11239)
@ehuss ehuss added this to the 1.66.0 milestone Oct 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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