Some Clippy suggestions #5070
Some Clippy suggestions #5070
Conversation
|
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
|
☔ The latest upstream changes (presumably #4978) made this pull request unmergeable. Please resolve the merge conflicts. |
|
rebased |
|
☔ The latest upstream changes (presumably #5044) made this pull request unmergeable. Please resolve the merge conflicts. |
|
rebased |
src/cargo/core/registry.rs
Outdated
| @@ -454,7 +454,7 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { | |||
| // If we don't have an override then we just ship | |||
| // everything upstairs after locking the summary | |||
| (None, Some(source)) => { | |||
| for patch in patches.iter() { | |||
| for patch in &patches { | |||
There was a problem hiding this comment.
Historically, I believe @alexcrichton preferred the .iter form, because it is more explicit. I personally also favor .iter().
However this definitely contradicts clippy's default, and there's certain benefit in sticking with defaults...
@alexcrichton do you have any plan here? Should we
- Persuade clippy folks that
.iter()is better and switch clippy's default :) - Give up on our preferences and just switch from
.iterto&everywhere? - Configure
clippyin Cargo.toml, so that it does not give this warning - Continue to reject PRs with this change once in a while
There was a problem hiding this comment.
Continue to reject PRs with this change once in a while
I am sorry. I should have checked whether this has come up before.
There was a problem hiding this comment.
Once this is decided I am happy to redo the other changes in this PR and re submit.
There was a problem hiding this comment.
Yeah I personally prefer .iter() as I find for x in &foo to be less clear and also slightly less readable, but again that's mostly personal preference
There was a problem hiding this comment.
@alexcrichton @Eh2406 let's try to add an exception for this rule to Cargo.toml than?
Although we don't run Clippy on Cargo on CI, it'll help people who do so locally!
There was a problem hiding this comment.
I will re do this pr without all of them, next time I have spare cycles. :-)
There was a problem hiding this comment.
Yeah I'd be fine adding an exception!
src/cargo/core/resolver/mod.rs
Outdated
| find_candidate(&mut backtrack_stack, | ||
| &mut cx, | ||
| &mut remaining_deps, | ||
| &mut parent, |
There was a problem hiding this comment.
Looks like the formatting got funky around here?
(cherry picked from commit 24836e9)
(cherry picked from commit de9a7b9)
|
Redone mostly by hand, sorry about the force push. This time with |
|
@bors: r+ |
|
📌 Commit 1ef675e has been approved by |
Some Clippy suggestions This is just some suggestions from Clippy and intellij rust. Clippy still has a lot to say, but this is some of the simple things.
|
☀️ Test successful - status-appveyor, status-travis |
| @@ -2,6 +2,11 @@ | |||
| #![cfg_attr(test, deny(warnings))] | |||
| #![recursion_limit="128"] | |||
|
|
|||
| // Currently, Cargo does not use clippy for its source code. | |||
| // But if someone runs it they should know that | |||
| // @alexcrichton disagree with clippy on some style things | |||
This is just some suggestions from Clippy and intellij rust.
Clippy still has a lot to say, but this is some of the simple things.