Skip to content
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

Correct default cargo new edition #9202

Merged
merged 4 commits into from
Mar 1, 2021
Merged

Correct default cargo new edition #9202

merged 4 commits into from
Mar 1, 2021

Conversation

gauntface
Copy link
Contributor

I think this is a small typo in the docs but feel free to close this PR if I'm mistaken

I think this is a small typo in the docs but feel free to close this PR if I'm mistaken
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 24, 2021
@ehuss
Copy link
Contributor

ehuss commented Feb 24, 2021

Hm, this has come up several times before (#8952, #7926). The default if the edition is not specified is 2015. cargo new fills in the edition field with the most recent one. Perhaps there is some way to make that a little clearer?

Suggestion for text when the edition isn't defined at all
Another alterations after pondering further
@gauntface
Copy link
Contributor Author

Ah I see what you mean now. I've changed my commit to be what would have made this more obvious for me but let me know if you have something else in mind.

Rationales for suggestions:

  1. Moving the example toml above the caveat sentence suggests that the final sentence is something different from cargo new's behavior
  2. Alter the wording to suggest that the default 2015 edition is used in specific scenarios. Originally I read the statement as the default value cargo new uses is 2015, which then was contradicted by the example.

As an FYI, this is me just going through the "Getting Started" sections of the docs and got curious about the edition field, so please feel free to close this PR if you'd prefer to follow up on this in a different forum with other folks. I just wanted to give some suggestions that would have helped me parse the docs as intended (or what I think was intended).

Based on feedback, removed invalid value statement.
@ehuss
Copy link
Contributor

ehuss commented Mar 1, 2021

Thanks! This looks clearer to me!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 1, 2021

📌 Commit d1c28a5 has been approved by ehuss

@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 Mar 1, 2021
@bors
Copy link
Contributor

bors commented Mar 1, 2021

⌛ Testing commit d1c28a5 with merge cb0e8c3...

@bors
Copy link
Contributor

bors commented Mar 1, 2021

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing cb0e8c3 to master...

@bors bors merged commit cb0e8c3 into rust-lang:master Mar 1, 2021
@alexcrichton
Copy link
Member

@gauntface out of curiosity, how did you come across this documentation and what led to your desire to fix it? I imagine there was confusion after reading the documentation, but I'm curious how you got on this page and what led you to it in the first place.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 3, 2021
Update cargo

12 commits in 572e201536dc2e4920346e28037b63c0f4d88b3c..c68432f1e5cbbc09833699a951b1b5b059651dff
2021-02-24 16:51:20 +0000 to 2021-03-02 18:26:29 +0000
- Don't panic when printing JSON with non-utf8 paths (rust-lang/cargo#9226)
- Detect changes for JSON spec targets. (rust-lang/cargo#9223)
- Fix `cargo_target_empty_cfg` test with env var. (rust-lang/cargo#9225)
- Correct default cargo new edition (rust-lang/cargo#9202)
- Update split-debuginfo docs around the default. (rust-lang/cargo#9224)
- Minor update to registry API error messages. (rust-lang/cargo#9213)
- Some minor code cleanup. (rust-lang/cargo#9214)
- doc: Fix spelling worksapce->workspace (rust-lang/cargo#9212)
- Update SPDX version in docs. (rust-lang/cargo#9209)
- Throw error if CARGO_TARGET_DIR is an empty string (rust-lang/cargo#8939)
- testsuite: Use split debuginfo on macos. (rust-lang/cargo#9207)
- testsuite: Improve performance when using rustup. (rust-lang/cargo#9206)
@gauntface
Copy link
Contributor Author

The path that lead me here was:

  1. Curious about rust and startef going through the getting started guide on the rust site
  2. Cargo new made the project
  3. The Cargo.toml file linked to this manifest document and I was curious what the "edition" field was, I read the sentence (briefly)
  4. Noticed my manifest had 2018 same as the sample and thought that "default to 2015" was a typo in the sentence above it.

I raised the PR assuming that it was a small thing that may have gone unnoticed.

@gauntface gauntface deleted the patch-1 branch March 3, 2021 15:26
@alexcrichton
Copy link
Member

Ok thanks for the info, that's what we suspected! I wordsmithed this a bit more at #9233 when we got another PR to update the wording, so I'm hoping that helps a bit more here too

@ehuss ehuss added this to the 1.52.0 milestone Feb 6, 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