rust-clippy
rust-clippy copied to clipboard
Fix/12035 silence struct field names
fixes #12035
Allows to silence lints struct_field_names
and enum_variant_names
through their impls.
This is done in order for some crates such as serde
to silence those lints
changelog: [struct_field_names
]: allow to silence lint through struct's impl
changelog: [enum_variant_names
]: allow to silence lint through enum's impl
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.
Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review
and S-waiting-on-author
) stays updated, invoking these commands when appropriate:
-
@rustbot author
: the review is finished, PR author should check the comments and take action accordingly -
@rustbot review
: the author is ready for a review, this PR will be queued again in the reviewer's queue
@bors r+
:pushpin: Commit 9a3cdbe4adf6614df2202307b1b5255332e1de45 has been approved by Manishearth
It is now in the queue for this repository.
:hourglass: Testing commit 9a3cdbe4adf6614df2202307b1b5255332e1de45 with merge 1ffad3acf864211bc81a61217f75758e22c99170...
:broken_heart: Test failed - checks-action_test
:hourglass: Testing commit 9a3cdbe4adf6614df2202307b1b5255332e1de45 with merge 8a2c2a2518bb0f967f667c7be1a80c2c25450ee7...
:broken_heart: Test failed - checks-action_dev_test
Hey @y21, this is a ping from triage. It looks like this PR was already approved. Is there anything that needs to be done, or did bors just fail for bors reasons?
Hi @xFrednet, after discussing a bit about the related issue on Zulip, it looks like it won't really be useful. So maybe we can close this PR and the related issue?
Hey @alexis-langlet, I've checked the tests and agree to close the issues. It seems a bit weird to me that impls should allow it, since the lints should trigger on the enum definition and not the impls.
Thank you for working on this and also being part of the discussion.
Okay, after reading skimming through the issue, I understand the motivation for allowing this lint to be allowed on an impl. My suggestion might be to allow traits to allow this lint instead, but it all seems a bit weird. I would like to get more input from the others.
I'll nominate this for the next meeting with the question:
- Do we want to allow this lint to be allowed on
impl
items or maybetrait
definitions?
@rustbot label +I-nominate
Personally, I think it's better to keep this lint simple. For a user it should be easy to add #[allow(..)]
to the enum. But since others already approved this PR, it's better to discuss this IMO
:umbrella: The latest upstream changes (presumably #12635) made this pull request unmergeable. Please resolve the merge conflicts.