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

Make sufficiently old or low-impact compatibility lints deny-by-default #36894

Merged
merged 1 commit into from
Oct 27, 2016

Conversation

petrochenkov
Copy link
Contributor

Tracking issues are updated/created when necessary.
Needs crater run before proceeding.

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Oct 4, 2016

Started crater run comparing 5045d4e and 21deca7fdbeb213ddb5d6356d1a542691d986fd1.

@nikomatsakis
Copy link
Contributor

Crater-report: https://gist.github.com/nikomatsakis/fc7c4951bc962b163395b7557e082f91

  • 5782 crates tested: 3741 working / 1355 broken / 19 regressed / 5 fixed / 662 unknown.

@petrochenkov
Copy link
Contributor Author

petrochenkov commented Oct 5, 2016

deny(invalid_type_param_default):

  • rust-freqdist-0.1.4 - defaults in impls
  • scan-rules-0.1.3 - defaults in impls
  • tangle-0.4.0 - defaults in impls
  • hex2d-dpcext-0.0.9 - defaults in fns
  • rotor-carbon-0.6.0 - defaults in fns

deny(illegal_struct_or_enum_constant_pattern):

  • elfloader32-0.0.3
  • elfloader-0.0.3
  • xhtmlchardet-1.0.0

deny(lifetime_underscore):

  • sxd-xpath-0.2.0
  • zoneinfo_parse-0.1.2

deny(inaccessible_extern_crate):

  • epoxy-0.0.2

illegal_floating_point_constant_pattern,
overlapping_inherent_impls,
super_or_self_in_global_path:

other failures are spurious


Seems reasonable to deny by default everything (except for, maybe, invalid_type_param_default).
I'll try to send PRs to affected crates today or tomorrow.

@nikomatsakis
Copy link
Contributor

@petrochenkov I made checkboxes and checked off the cases where you had sent PRs. The only one I didn't see was rust-freqdist, did I miss it?

@petrochenkov
Copy link
Contributor Author

@nikomatsakis
I didn't send PR to rust-freqdist because it was already fixed by the author, just unpublished.

@nikomatsakis nikomatsakis added T-core Relevant to the core team, which will review and decide on the PR/issue. and removed T-core Relevant to the core team, which will review and decide on the PR/issue. labels Oct 17, 2016
@brson brson added the relnotes Marks issues that should be documented in the release notes of the next release. label Oct 17, 2016
@nikomatsakis nikomatsakis added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 17, 2016
@nikomatsakis
Copy link
Contributor

Per my proposed rules in this thread, I move that we merge this PR, going from stage 1 to stage 2.

@nikomatsakis
Copy link
Contributor

@rfcbot fcp merge

I propose that we move this PR to stage 2: deny-by-default. There were <19 total affected crates, and all have been notified with pending PRs, except for one which was already fixed by the author.

@rfcbot
Copy link

rfcbot commented Oct 17, 2016

Team member nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.
Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@michaelwoerister
Copy link
Member

@rfcbot reviewed

So the rfcbot can (and should) be used for regular PRs too, not just RFCs?

@nikomatsakis
Copy link
Contributor

@michaelwoerister

So the rfcbot can (and should) be used for regular PRs too, not just RFCs?

indeed -- also issues.

@nikomatsakis
Copy link
Contributor

I tagged @Aatch, @bkoropoff, and @dotdash since none of them have been very active of late.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 25, 2016

📌 Commit fc3255f has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 26, 2016

⌛ Testing commit fc3255f with merge ea45430...

@bors
Copy link
Contributor

bors commented Oct 26, 2016

💔 Test failed - auto-linux-64-opt

@petrochenkov
Copy link
Contributor Author

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 26, 2016

📌 Commit f9b9a69 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 27, 2016

☔ The latest upstream changes (presumably #36695) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Contributor Author

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 27, 2016

📌 Commit 2a85211 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Oct 27, 2016

⌛ Testing commit 2a85211 with merge a0e31a5...

bors added a commit that referenced this pull request Oct 27, 2016
Make sufficiently old or low-impact compatibility lints deny-by-default

Tracking issues are updated/created when necessary.
Needs crater run before proceeding.

r? @nikomatsakis
@bors bors merged commit 2a85211 into rust-lang:master Oct 27, 2016
@petrochenkov petrochenkov deleted the deny branch March 16, 2017 19:40
bors added a commit that referenced this pull request Jun 1, 2017
Turn sufficiently old compatibility lints into hard errors

It's been almost 7 months since #36894 was merged, so it's time to take the next step.

[breaking-change], needs crater run.

PRs/issues submitted to affected crates:
https://github.com/alexcrichton/ctest/pull/17
Sean1708/rusty-cheddar#55
m-r-r/helianto#3
azdle/virgil#1
rust-locale/rust-locale#24
mneumann/acyclic-network-rs#1
reem/rust-typemap#38

cc https://internals.rust-lang.org/t/moving-forward-on-forward-compatibility-lints/4204
cc #34537 #36887
Closes #36886
Closes #36888
Closes #36890
Closes #36891
Closes #36892
r? @nikomatsakis
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants