-
Notifications
You must be signed in to change notification settings - Fork 2.8k
docs(report): enhance man pages for cargo report *
#16430
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
base: master
Are you sure you want to change the base?
Conversation
e08f161 to
eb13e52
Compare
d34d267 to
870a35a
Compare
870a35a to
9c693b6
Compare
I'm a bit concerned about this as |
| @@ -0,0 +1,64 @@ | |||
| # cargo-report-future-incompat(1) | |||
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.
Do we need an FCP for this as well?
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.
This is a doc change that we can easily revert by redirecting the new page to https://doc.rust-lang.org/nightly/cargo/reference/future-incompat-report.html, so I don't think it need an FCP.
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.
Don't remember what my concern was here
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.
Now I remember: do we have compatibility guarantees on the man pages that we provide?
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.
Ah, for that, yes. The toolchain distribution tarball has them in ./share/man/man1. Also rustup man <dot-joined-cmd> displays them.
So in terms of CLI compatibility we might want an FCP.
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.
The toolchain distribution tarball has them in ./share/man/man1
Some OS distros include them in man database when install a toolchain from the system package manager.
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.
Before starting FCP, let's align with the concerns.
The major change of this is we expose man pages and documentation of sub-subcommand (i.e., cargo report future-incompatibilities).
- If we consider man pages for human only, it is less an concern. If we then don't want to expose sub-subcommand later, we can just redirect them to their parent command.
- If we consider man pages also for manchine/programmable use cases, then we need to be ensure what the compatibility we guarantee here before making this change.
- Do we always want sub-subcommands to have its own man pages, or case-by-case? For example,
cargo worktreedoesn't have man pages forworktree add|removeseparately This is similar to ourcargo ownercase. OTOH,docker container ls|rm|inspectdoes, as all the sub-subcommands are different than others.
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.
Do we always want sub-subcommands to have its own man pages, or case-by-case? For example, cargo worktree doesn't have man pages for worktree add|remove separately This is similar to our cargo owner case. OTOH, docker container ls|rm|inspect does, as all the sub-subcommands are different than others.
I think this should be handled on a case by case basis. Some subcommands are modes of the parent subcommand, like git stash. Clap has support for this as flatten_help. Other subcommands are more independent of each other and should have independent help.
9c693b6 to
8a7ab6e
Compare
This comment has been minimized.
This comment has been minimized.
8a7ab6e to
01940e6
Compare
Removed. |
01940e6 to
f895fd7
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
f895fd7 to
5f18a45
Compare
This comment has been minimized.
This comment has been minimized.
a81c2b9 to
511422f
Compare
|
@rfcbot fcp merge T-cargo This proposes to generate man pages of Affected UI:
Depending on our compatibility guarantee around man pages, this can be seen as one-way door or two-way door decision:
|
|
Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
FWIW, my intention was to handle this on a case-by-case basis. I suspected |
This comment has been minimized.
This comment has been minimized.
511422f to
9c546e2
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
### What does this PR try to resolve? docs(unstable): expand docs for `-Zbuild-analysis` ### How to test and review this PR? [rendered](https://github.com/weihanglo/cargo/blob/ed66196b0b0e71aff8208565533411ffde49ff8a/src/doc/src/reference/unstable.md) Each command's doc will be in their own man page after this is merged <#16430>.
FCP
What does this PR try to resolve?
Make space for future report kinds iun
cargo reportdocumentations.cargo reportman page didn't match its--helptext equivalentcargo report future-incompatdidn't have its own man page.Note that
cargo helphasn't yet supported forfuture-incompat.It will be a separate PR.
How to test and review this PR?