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

add long explanation for E0453, lint attribute overruled by outer forbid #34242

Merged
merged 2 commits into from Jun 13, 2016

Conversation

zackmdavis
Copy link
Member

This is a subtask of #32777.


r? @GuillaumeGomez

@rust-highfive
Copy link
Collaborator

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


#[allow(non_snake_case)]
fn main() {
let MyNumber = 2; // error: allow(non_snake_case) overruled by outer
Copy link
Member

Choose a reason for hiding this comment

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

Please indent the error comment like this:

    let MyNumber = 2; // error: allow(non_snake_case) overruled by outer
                      //        forbid(non_snake_case)

@GuillaumeGomez
Copy link
Member

Thanks for your PR! Just fix the two issues and it'll be good to go!

It turns out that the subsequent lines of the error message comment
should be aligned like this.

The "turns the corresponding compiler warning" language may not be
strictly the most accurate—a lint check isn't the same as a compiler
warning; it emits a compiler warning if it's set to the `warn` level—
but it may be worth glossing over such distinctions in favor of simple,
familar phrasings for the sake of pedagogy; thanks to Guillaume Gomez
for the wording suggestion.

Let's also fix up the introductory clauses of the sentences about how to
fix the error to put a little more emphasis on the fact that the
`forbid` setting was probably there for a reason.
@zackmdavis
Copy link
Member Author

(In addition to addressing the two reviewer comments, the new commit also edits the introductory clauses of the how-to-fix-it sentences to be more cautious about recommending weakening forbid to deny.)

@GuillaumeGomez
Copy link
Member

All good for me. Thanks @zackmdavis!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jun 13, 2016

📌 Commit e4c566c has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Jun 13, 2016

⌛ Testing commit e4c566c with merge c0df447...

bors added a commit that referenced this pull request Jun 13, 2016
add long explanation for E0453, lint attribute overruled by outer forbid

This is a subtask of #32777.

-----

r? @GuillaumeGomez
@bors bors merged commit e4c566c into rust-lang:master Jun 13, 2016
@zackmdavis zackmdavis deleted the explain_E0453 branch June 13, 2016 17:29
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.

None yet

4 participants