-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
docs(reference): sort manifest package section keys per style guide #15285
base: master
Are you sure you want to change the base?
Conversation
https://doc.rust-lang.org/nightly/style-guide/cargo.html Sorting only, no content changes.
r? @weihanglo rustbot has assigned @weihanglo. Use |
* [`edition`](#the-edition-field) --- The Rust edition. | ||
* [`rust-version`](rust-version.md) --- The minimal supported Rust version. | ||
* [`description`](#the-description-field) --- A description of the package. | ||
* [`autobenches`](cargo-targets.md#target-auto-discovery) --- Disables bench auto discovery. |
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.
For example, personally, autobenches
is definitely not as important as other fields like edition
, rust-version
, and description
. This might make the reading experience slightly worse for readers who go through it from top to bottom.
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.
FWIW, I agree with this, the ordering in the cargo docs seems better to me.
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.
Thanks for the pull request!
This is a good attempt, though the ownership of the style of Carg.toml doesn't seem clear to me right now.
As a consequence, we see some issues like this one:
I am not sure if the style guide is settled, so maybe we should postpone this until things get clearer?
@@ -135,47 +135,84 @@ authors = ["Graydon Hoare", "Fnu Lnu <[email protected]>"] | |||
This field is surfaced in package metadata and in the `CARGO_PKG_AUTHORS` | |||
environment variable within `build.rs` for backwards compatibility. | |||
|
|||
### The `edition` field | |||
## The `[badges]` section |
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.
Lexicographic order makes sense for a reference-like documentation. I believe we did it for .cargo/config.toml
docs. However, I feel like fields in configuration are mostly equally-important but in Cargo.toml they are not. Moving an unused field upfront doesn't seem too good.
Anyway, this is not a suggestion for you to apply. I am just sharing an example that I don't fully agree with the style guide.
cc @joshtriplett, what is your thought on this? |
@weihanglo This portion of styling has not been finalized and does not have consensus with the Cargo team, so it's not ready to be acted on. The fact that this prompted a PR means we should remove this portion of the style guide until we have a new version that has consensus with Cargo. |
JFYI there's tooling in existence that is already making use of the style guide sort order, which kind of led to me opening this PR: dprint/dprint-plugin-toml#29 This is not to say I'd personally lean towards the style guide on this (I don't -- the one in cargo docs seem "better" to me), but as said, JFYI. |
https://doc.rust-lang.org/nightly/style-guide/cargo.html
Sorting only, no content changes.