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

Edition key should be per-target, not per-package #5816

Merged
merged 7 commits into from
Jul 31, 2018

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Jul 27, 2018

Fixes #5661

I've pushed this WIP PR as I'd love some early feedback on it and some tips on:

  • how to best to make it fail if edition is set on a target, but the feature isn't set; and
  • what tests this should include (i.e how exhaustive should I go)

Thanks!

@rust-highfive
Copy link

r? @matklad

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

@alexcrichton
Copy link
Member

Nice! I think the test for the feature being enabled here is similar to the main feature gate for the edition key, basically only activate it if the edition specified is Some. Otherwise I think it's fine to just have a few tests in this regard to make sure that different editions work (and that they can be specified)

@alexcrichton
Copy link
Member

Oh also, can the documentation be updated too?

Test:
* enabling edition feature & setting at target level (happy path)
* overriding the package-level edition with per-target edition
* feature gating of per-target edition
* per-target edition usage for rustdoc
@alexcrichton alexcrichton changed the title [WIP] Edition key should be per-target, not per-package Edition key should be per-target, not per-package Jul 31, 2018
@alexcrichton
Copy link
Member

Looks fantastic, thanks so much @dwijnand!

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 31, 2018

📌 Commit 67c52ff has been approved by alexcrichton

bors added a commit that referenced this pull request Jul 31, 2018
Edition key should be per-target, not per-package

Fixes #5661

I've pushed this WIP PR as I'd love some early feedback on it and some tips on:

* how to best to make it fail if edition is set on a target, but the feature isn't set; and
* what tests this should include (i.e how exhaustive should I go)

Thanks!
@bors
Copy link
Contributor

bors commented Jul 31, 2018

⌛ Testing commit 67c52ff with merge 4b95e8c...

@bors
Copy link
Contributor

bors commented Jul 31, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 4b95e8c to master...

@bors bors merged commit 67c52ff into rust-lang:master Jul 31, 2018
@dwijnand dwijnand deleted the edtion-per-target branch July 31, 2018 19:35
@ehuss ehuss mentioned this pull request Mar 26, 2019
bors added a commit that referenced this pull request Mar 26, 2019
Some fingerprint cleanup.

Just a minor cleanup.

Move `CARGO_PKG_*` values from Metadata to Fingerprint (added in #3857).  Closes #6208.  This prevents stale artifacts from being left behind when these values change. Also tracks changes to the "repository" value (added in #6096).

Remove `edition` as a separate field. It is already tracked in `target`. This was required previously to #5816 which added per-target editions.

Also adds a helper to the testsuite to make globbing easier.
@ehuss ehuss added this to the 1.29.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.

Edition key should be per-target, not per-package
6 participants