assigning_clones: move to pedantic so it is allow by default
#12779
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @y21 (or someone else) some time within the next two weeks. 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 (
|
rustbot
added
the
S-waiting-on-review
de-vri-es
force-pushed
the
move-assigng-clones-to-pedantic
branch
from
a87bba3
to
37808d0
Compare
de-vri-es
changed the title
assigning_clone: move to pedantic so it is allow by default
de-vri-es
changed the title
assigning_clone: move to pedantic so it is allow by defaultassigning_clones: move to pedantic so it is allow by default
de-vri-es
force-pushed
the
move-assigng-clones-to-pedantic
branch
from
37808d0
to
076f2e5
Compare
|
I opened a thread on zulip to discuss this first and see if others have strong preference on the category and want to wait it out a bit: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/assigning_clones.20lint.20category/near/437732612 |
y21
added
the
S-needs-discussion
|
possible other move: knock it back to the nursery for a bit? |
|
The lint's author has already moved it to nursery after the first couple issues to work a little bit more on the lint. I'm sure of it, I reviewed the PR weeks ago. I'm not sure if something happened on the beta backporting process, or maybe the commit didn't go through? |
|
oh okay! it is plausible I am just slightly oblivious. |
|
Or maybe I'm just having some Mandela effect, because I just checked Kobzol's PR history on this repo and he didn't create that PR... And there doesn't seem to exist any changes on the blame for that specific section. We should definitely move it to |
|
Most of the issues already have a fix PR up, but I'm also fine with moving this to nursery, at least for the time being. Looking at #12778 again and issues/PRs from other repos that link there, it does seem like people agree with that this lint regresses readability and it seems like the initial idea was for this lint to be in pedantic anyway according to the zulip meeting a year ago when this was first discussed, so we'll probably want to have it at most in pedantic too once the issues are sorted out. |
|
I think all those fixes are thanks to it being a warn-by-default lint now :p Sounds like it doesn't necessarily need to go back to the nursery anymore. Maaaybe there should be a |
There was a problem hiding this comment.
Yeah this seems fine, there seems to be generally more people that agree that it hurts readability than not from the few reactions that I've seen. Let's get this merged now so that it makes it into the sync in time and wouldn't need to wait another 2 weeks to get to nightly.
I agree that moving nursery might not be necessary anymore with those fixes up. The remaining ones that don't have a PR fixing it are mostly similar subjective issues (e.g. the fact it's not useful in test code, which should not be so bad for a pedantic lint (but are still ones that should be fixed of course))
|
Thanks. @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
In a nutshell, the
assigning_cloneslint suggests to make your code less readable for a small performance gain. See #12778 for more motivation.fixes #12778
changelog: [
assigning_clones]: move to thepedanticgroup