Last round of suggestions by clippy #403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These were flagged by `cargo clippy`:
warning: redundant field names in struct initialization
There are plenty more redundant field names, but I only changed the ones
where the initialization was a single line of code. I still prefer the
redundant style for multi-line initializations (and I'm under the
impression that others agree), so I've also disabled the warning.
These were flagged by `cargo clippy`:
warning: this `else { if .. }` block can be collapsed
These were flagged by `cargo clippy`:
warning: returning the result of a `let` binding from a block
warning: this import is redundant
These were flagged by `cargo clippy`:
warning: the loop variable is used to index
I've verified that this doesn't increase the size of consuming binaries.
Pretty impressive. I tested this with a project that uses the following
features: ethernet, proto-dhcpv4, socket-tcp.
whitequark
reviewed
Dec 29, 2020
whitequark
reviewed
Dec 29, 2020
whitequark
requested changes
Dec 29, 2020
These were flagged by `cargo clippy`:
warning: the operation is ineffective. Consider reducing it to `number`
warning: this function has too many arguments (8/7)
warning: you should consider adding a `Default` implementation for
`phy::loopback::Loopback`
I like the code better as it is.
This was flagged by `cargo clippy`:
warning: called `map(f)` on an `Option` value where `f` is a closure
that returns the unit type `()`
I decided to disable this because it was suggesting changes like the
following and I prefer the original:
@@ -1 +1 @@
-self.waker.take().map(|w| w.wake());
+if let Some(w) = self.waker.take() { w.wake() }
|
I want to double check that the clippy check actually fails this time. I've got one final commit to address the last lint. |
whitequark
approved these changes
Dec 29, 2020
These were flagged by `cargo clippy`:
warning: using `clone` on a `Copy` type
This was flagged by `cargo clippy`:
warning: redundant closure found
|
Today I learned about
That's why the job isn't failing. I'm reasonably confident this will work as-is. |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-on to #402 and is the last in the series. Now that all of the warnings have been addressed, I've also set
cargo clippyto fail when it encounters warnings going forward.Should we pin to a specific toolchain or do folks think it's feasible to keep up-to-date with the latest stable? I don't have a sense for how often new lints are added.