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

Stabilize edition key and add cargo new --edition #5984

Merged
merged 1 commit into from
Sep 6, 2018

Conversation

alexcrichton
Copy link
Member

This commit stabilizes the edition key in Cargo.toml, both in the
[package] section and inside subtargets. Additionally the cargo new and
cargo init subcommands have been enhanced with a --edition flag to allow
explicitly specifying the edition to be generated.

This commit does not yet change the default edition that's generated.

Closes #5980

@rust-highfive
Copy link

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

r? @ehuss

cc @Mark-Simulacrum

@rust-highfive rust-highfive assigned ehuss and unassigned matklad Sep 5, 2018
@dwijnand
Copy link
Member

dwijnand commented Sep 5, 2018

Tweak (or drop?) the section in src/doc/src/reference/unstable.md?

let manifest = fs::read_to_string(paths::root().join("foo/Cargo.toml")).unwrap();
assert!(manifest.contains("edition = \"2018\""));
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A 'what is the default edition' test would be good too -- it should be 2018 I believe.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see that you didn't change the default edition, is there a reason for that? I would semi-expect us to do that.

cc @Centril @aturon

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes agreed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand, the team discussed this and agreed not to do this early for nightly and just wait for another cycle

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at that document provides no clear rationale to me unless I missed it - the plan is that RC1 is a fully edition 2018 compatible compiler - and making that edition the default for new projects makes sense then.

Could someone from the cargo team perhaps clear up why we didn't want to change the default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The conclusion stemmed from being conservative. The compiler isn't ready right-the-red-hot-second for --edition 2018 to be the default because there's still unstable features (like the module system) which are in the process of stabilizing. As soon as the compiler is ready we'll change, and we conservatively said that "just before October 25 this must happen"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcrichton right but this PR is part of several ones in the queue that will stabilize all the pieces.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok! It does indeed look like everything is in motion now. I'll make a PR to change the default after this one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've submitted #5989 which switches to generating 2018 by default.

@Mark-Simulacrum
Copy link
Member

I also think we should land this now and upstream into rust-lang/rust, we can discuss and land the default change separately since it seems like we don't have consensus there.

This commit stabilizes the `edition` key in `Cargo.toml`, both in the
`[package]` section and inside subtargets. Additionally the `cargo new` and
`cargo init` subcommands have been enhanced with a `--edition` flag to allow
explicitly specifying the edition to be generated.

This commit does not yet change the default edition that's generated.

Closes rust-lang#5980
@alexcrichton
Copy link
Member Author

@dwijnand an excellent point! The docs should be updated now (and tests hopefully passing)

@ehuss
Copy link
Contributor

ehuss commented Sep 6, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Sep 6, 2018

📌 Commit 3d02903 has been approved by ehuss

@bors
Copy link
Contributor

bors commented Sep 6, 2018

⌛ Testing commit 3d02903 with merge 56ee620...

bors added a commit that referenced this pull request Sep 6, 2018
Stabilize `edition` key and add `cargo new --edition`

This commit stabilizes the `edition` key in `Cargo.toml`, both in the
`[package]` section and inside subtargets. Additionally the `cargo new` and
`cargo init` subcommands have been enhanced with a `--edition` flag to allow
explicitly specifying the edition to be generated.

This commit does not yet change the default edition that's generated.

Closes #5980
@bors
Copy link
Contributor

bors commented Sep 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: ehuss
Pushing 56ee620 to master...

@bors bors merged commit 3d02903 into rust-lang:master Sep 6, 2018
@alexcrichton alexcrichton deleted the stabilize-edition branch September 7, 2018 00:02
@ehuss ehuss added this to the 1.30.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo new --edition 2018?
9 participants