-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Clarify cargo manifest edition field docs #8953
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Eh2406 (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
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.
Overall I like this, just one word-smithing nit.
src/doc/src/reference/manifest.md
Outdated
default to 2015. | ||
The `edition` key is an optional key that affects which edition your package | ||
is compiled with. Cargo will always generate packages via [`cargo new`] with | ||
the `edition` key set to the latest edition. Setting the `edition` key in |
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.
Cargo will always generate packages via [
cargo new
] with
theedition
key set to the latest edition.
Perhaps:
[cargo new
] will generate a package with the edition
key set to the latest edition.
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, I like that wording much better. see my new changes!
thanks for the review @Eh2406 , see my updated wordsmithing changes :) |
I think it is good, but want @ehuss's opinion before we merge. I have messed this up before. :-P |
Looks great, thanks! |
📌 Commit da5b079 has been approved by |
☀️ Test successful - checks-actions |
Update cargo 10 commits in 63d0fe43449adcb316d34d98a982b597faca4178..d274fcf862b89264fa2c6b917b15230705257317 2020-12-02 01:44:30 +0000 to 2020-12-07 23:08:44 +0000 - Clarify cargo manifest edition field docs (rust-lang/cargo#8953) - Run rustdoc doctests relative to the workspace (rust-lang/cargo#8954) - Workaround fs issue in `cargo publish`. (rust-lang/cargo#8950) - Fix panic with -Zbuild-std and no roots. (rust-lang/cargo#8942) - Slightly optimize `cargo vendor` (rust-lang/cargo#8937) - Fixes rust-lang/cargo#8783 , cargo new fails without a author name or email (rust-lang/cargo#8912) - Fix test escaping __CARGO_TEST_ROOT (rust-lang/cargo#8929) - Add period to allowed feature name characters. (rust-lang/cargo#8932) - faq: small fixes (rust-lang/cargo#8931) - Fix semver documentation tests. (rust-lang/cargo#8930)
addresses #8951
This PR aims to clarify the documentation for the
edition
field in the Cargo manifest.The confusion (IME) stems from the behavior of
cargo new
(defaults to writing edition = "2018") being confused for the default behavior of how Cargo interprets the manifest (edition
is an optional key, defaults to 2015).would love to get some other thoughts on how we can clarify this since it seems like I'm not the only one who got confused @Eh2406