Skip to content

diagnostically note source of overruling outer forbid #34251

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

Merged
merged 1 commit into from
Jul 31, 2016

Conversation

zackmdavis
Copy link
Member

When we emit E0453 (lint level attribute overruled by outer forbid
lint level), it could be helpful to note where the forbid level was
set, for the convenience of users who, e.g., believe that the correct
fix is to weaken the forbid to deny.

forbidden_on_whose_authority

@rust-highfive
Copy link
Contributor

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

@zackmdavis
Copy link
Member Author

CI reports that lint-plugin-forbid-attrs.rs fails, which is strange, because I thought everything passed when I ran the tests locally (do I need to make clean before I make check or something?).

And the error is that the new note-level subdiagnostic was unexpected, which is also strange because I did add an expectation comment—does the same-line form not work with NOTE?? We shall wait and see (5a23319).

@zackmdavis
Copy link
Member Author

CI continues to fail; I'll have to investigate more some other day.

@bors
Copy link
Collaborator

bors commented Jul 12, 2016

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

@alexcrichton
Copy link
Member

Oh dear, sorry for the delay @zackmdavis! @Manishearth or @jonathandturner, would one of you be willing to take a look?

@Manishearth
Copy link
Member

Could you rebase this and see what CI thinks of it then? I'll review after.

@Manishearth Manishearth assigned Manishearth and unassigned Aatch Jul 19, 2016
@zackmdavis zackmdavis force-pushed the forbidden_on_whose_authority branch from 5a23319 to 6350011 Compare July 19, 2016 14:08
@zackmdavis
Copy link
Member Author

(rebased)

@Manishearth
Copy link
Member

The failure is legitimate, it is lint level defined here, not "forbid lint level ...". The "lint level defined here" comes from the actual lint being triggered in that test (the other test doesn't trigger any lints).

Just add "NOTE lint level defined here" to line 16 and you should be good

When we emit E0453 (lint level attribute overruled by outer `forbid`
lint level), it could be helpful to note where the `forbid` level was
set, for the convenience of users who, e.g., believe that the correct
fix is to weaken the `forbid` to `deny`.
@zackmdavis zackmdavis force-pushed the forbidden_on_whose_authority branch from 6350011 to 661b4f0 Compare July 30, 2016 22:19
@zackmdavis
Copy link
Member Author

@Manishearth thanks! (force-pushed)

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 31, 2016

📌 Commit 661b4f0 has been approved by Manishearth

@bors
Copy link
Collaborator

bors commented Jul 31, 2016

⌛ Testing commit 661b4f0 with merge 30b0a45...

bors added a commit that referenced this pull request Jul 31, 2016
…ishearth

diagnostically note source of overruling outer forbid

When we emit E0453 (lint level attribute overruled by outer `forbid`
lint level), it could be helpful to note where the `forbid` level was
set, for the convenience of users who, e.g., believe that the correct
fix is to weaken the `forbid` to `deny`.

![forbidden_on_whose_authority](https://cloud.githubusercontent.com/assets/1076988/15995312/2d847376-30ce-11e6-865e-b68cfebc0291.png)
@bors
Copy link
Collaborator

bors commented Jul 31, 2016

💔 Test failed - auto-win-gnu-32-opt

@zackmdavis
Copy link
Member Author

Glancing at the buildbot log, I don't see any individual test failures; the relevant part seems to be " command timed out: 1200 seconds without output, attempting to kill". I don't know what to do about this.

@Manishearth
Copy link
Member

@bors retry

  • CI timeout

@bors
Copy link
Collaborator

bors commented Jul 31, 2016

⌛ Testing commit 661b4f0 with merge 379ac50...

bors added a commit that referenced this pull request Jul 31, 2016
…ishearth

diagnostically note source of overruling outer forbid

When we emit E0453 (lint level attribute overruled by outer `forbid`
lint level), it could be helpful to note where the `forbid` level was
set, for the convenience of users who, e.g., believe that the correct
fix is to weaken the `forbid` to `deny`.

![forbidden_on_whose_authority](https://cloud.githubusercontent.com/assets/1076988/15995312/2d847376-30ce-11e6-865e-b68cfebc0291.png)
@bors bors merged commit 661b4f0 into rust-lang:master Jul 31, 2016
@zackmdavis zackmdavis deleted the forbidden_on_whose_authority branch July 31, 2016 18:47
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.

6 participants