Conversation
|
|
||
| p.cargo("check") | ||
| .masquerade_as_nightly_cargo(&["feature-metadata"]) | ||
| .run(); |
There was a problem hiding this comment.
Should there be a check of the new key being parsed?
There was a problem hiding this comment.
I guess this can similarily be split into two (test, fix) commits, so that we don't need a verification but the test diff showing that it was an unused manifest key.
There was a problem hiding this comment.
If we had a cargo metadata integration, then we'll have a verification for the consumption side (yeah we should continue the design discussion).
Or we can also add cargo-info integration for feature metadata in a follow-up as part of the verification. @0xPoe any opinion on this? I am not sure whether it should be behind a unstable flag though, given cargo-info is purely for human not programmable.
There was a problem hiding this comment.
I guess this can similarily be split into two (test, fix) commits, so that we don't need a verification but the test diff showing that it was an unused manifest key.
I've split the PR: the feature_has_documentation test case is now introduced in the first commit, triggers the "unused key" warning, which then disappears in the second commit as support for the key is added.
There was a problem hiding this comment.
Or we can also add cargo-info integration for feature metadata in a follow-up as part of the verification. @0xPoe any opinion on this? I am not sure whether it should be behind a unstable flag though, given cargo-info is purely for human not programmable.
Sounds interesting. I believe docs may also be helpful for human use. However, the biggest challenge would be the UI design. In the current cargo-info output, it is difficult to display the documents directly within the existing layout.
e5b1573 to
0baabe5
Compare
|
The CI failure seems unrelated. |
|
r? @weihanglo rustbot has assigned @weihanglo. Use Why was this reviewer chosen?The reviewer was selected based on:
|
There was a problem hiding this comment.
Thanks!
Could you expand https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#feature-metadata a bit? Perhaps with a subsection linking to feature-documentation RFC and tracking issue.
(Also just noticed that we forgot to put feature-metadata RFC link there)
| /// Features that this feature enables. | ||
| pub enables: Vec<String>, | ||
|
|
||
| /// Unstable feature `feature-documentation`. Documentation for the feature. |
There was a problem hiding this comment.
| /// Unstable feature `feature-documentation`. Documentation for the feature. | |
| /// Documentation for the feature. |
I think this should be sufficient?
|
|
||
| p.cargo("check") | ||
| .masquerade_as_nightly_cargo(&["feature-metadata"]) | ||
| .run(); |
There was a problem hiding this comment.
I guess this can similarily be split into two (test, fix) commits, so that we don't need a verification but the test diff showing that it was an unused manifest key.
|
|
||
| p.cargo("check") | ||
| .masquerade_as_nightly_cargo(&["feature-metadata"]) | ||
| .run(); |
There was a problem hiding this comment.
If we had a cargo metadata integration, then we'll have a verification for the consumption side (yeah we should continue the design discussion).
Or we can also add cargo-info integration for feature metadata in a follow-up as part of the verification. @0xPoe any opinion on this? I am not sure whether it should be behind a unstable flag though, given cargo-info is purely for human not programmable.
0baabe5 to
5d1f882
Compare
5d1f882 to
9199a6e
Compare
There was a problem hiding this comment.
Looks good to me. Let me know if you want to address #17447 (comment) or I can tweak a bit for you :)
aee401e to
223796b
Compare
Update cargo submodule 6 commits in 7941be6fb416b4cd9666aef7b858dfea25587a8c..30754e087c97c58c6c36d0440d4cf1aecb3330a9 2026-09-11 20:47:37 +0000 to 2026-09-15 18:19:50 +0000 - fix(git): For git cli, tell users what config we aren't forwarding on error (rust-lang/cargo#17477) - Lower the lint level of `manual_readme` and `non_kebab_case_bins` to `allow` (rust-lang/cargo#17478) - fix(trim-paths)!: unremap file in one JSON doc (rust-lang/cargo#17476) - feat(manifest): support `feature-documentation` in manifests (rust-lang/cargo#17447) - chore: Add Muscraft to the review rotation (rust-lang/cargo#17471) - Specify `--edition` in `messages` tests (rust-lang/cargo#17470) r? Urgau for the debugger script part --- 🤖 LLM disclosure: I probably should have tested this change manually, though I let LLM verify it working, sorry. Will do a manual test when I have time or if somebody beats me on that.
Update cargo submodule 9 commits in 7941be6fb416b4cd9666aef7b858dfea25587a8c..495c385d0875c4ba51eb72ea0448a2d4c018b8d4 2026-09-11 20:47:37 +0000 to 2026-09-16 12:48:58 +0000 - Report the number of errors with `build.warnings='deny'` (rust-lang/cargo#17479) - fix: return correct package specs when resolving workspace deps (rust-lang/cargo#17469) - refactor(config): Consolidate more config table definitions (rust-lang/cargo#17480) - fix(git): For git cli, tell users what config we aren't forwarding on error (rust-lang/cargo#17477) - Lower the lint level of `manual_readme` and `non_kebab_case_bins` to `allow` (rust-lang/cargo#17478) - fix(trim-paths)!: unremap file in one JSON doc (rust-lang/cargo#17476) - feat(manifest): support `feature-documentation` in manifests (rust-lang/cargo#17447) - chore: Add Muscraft to the review rotation (rust-lang/cargo#17471) - Specify `--edition` in `messages` tests (rust-lang/cargo#17470) r? Urgau for the debugger script part --- 🤖 LLM disclosure: I probably should have tested this change manually, though I let LLM verify it working, sorry. Will do a manual test when I have time or if somebody beats me on that.
Update cargo submodule 9 commits in 7941be6fb416b4cd9666aef7b858dfea25587a8c..495c385d0875c4ba51eb72ea0448a2d4c018b8d4 2026-09-11 20:47:37 +0000 to 2026-09-16 12:48:58 +0000 - Report the number of errors with `build.warnings='deny'` (rust-lang/cargo#17479) - fix: return correct package specs when resolving workspace deps (rust-lang/cargo#17469) - refactor(config): Consolidate more config table definitions (rust-lang/cargo#17480) - fix(git): For git cli, tell users what config we aren't forwarding on error (rust-lang/cargo#17477) - Lower the lint level of `manual_readme` and `non_kebab_case_bins` to `allow` (rust-lang/cargo#17478) - fix(trim-paths)!: unremap file in one JSON doc (rust-lang/cargo#17476) - feat(manifest): support `feature-documentation` in manifests (rust-lang/cargo#17447) - chore: Add Muscraft to the review rotation (rust-lang/cargo#17471) - Specify `--edition` in `messages` tests (rust-lang/cargo#17470) r? Urgau for the debugger script part --- 🤖 LLM disclosure: I probably should have tested this change manually, though I let LLM verify it working, sorry. Will do a manual test when I have time or if somebody beats me on that.
What does this PR try to resolve?
This implements part of RFC
feature-documentation, tracked by #17445. This only adds support for the new key in the schema. This doesn't add anything to the index as this was decided against.This doesn't not introduce a dedicated unstable feature, as mentioned in #17445 (comment), and the new key is simply gated behind the existing
feature-metadataunstable feature.The exact name of the key is still TBD, but it will be easy to rename either in this PR or later.
Open questions
feature-metadatathere, or stacked as a separate PR on top of docs(book): document the table syntax of features #17446?Future work
At least the following is still to be done for this RFC (I may not have time to tackle this):
cargo metadata.cargo add(and through other tools/services, like docs.rs).How to test and review this PR?
The tests should be enough to review this.