chore: clippy nursery#535
Conversation
StuartHarris
force-pushed
the
clippy-nursery
branch
from
a7982bd to
8799bbd
Compare
|
Nice ! I would suggest adding the lints in the |
charypar
left a comment
charypar
left a comment
There was a problem hiding this comment.
Not sure I agree with the push for map_or_ – seems to hurt readability – but don't have a strong opinion (and forced consistency is nice.)
|
|
||
| if let Some(cmd) = self.command.as_mut() { | ||
| #[allow(clippy::needless_collect)] | ||
| for event in cmd.events().collect::<Vec<_>>() { |
There was a problem hiding this comment.
I wonder whether something like .cloned() would be better and not trigger clippy...?
There was a problem hiding this comment.
It would, but Event is not Clone. We can't avoid the collect, because we're borrowing self through cmd, so we then can't mutably borrow it again inside the loop. But I can convince clippy that the collect is load-bearing by extracting to a variable. Gonna do that.
| /// `FakeShell` implements `EffectSender` for use in our internal tests. | ||
| #[derive(Clone, Default)] | ||
| pub(crate) struct FakeShell { | ||
| pub struct FakeShell { |
There was a problem hiding this comment.
I personally don't like this clippy, I think it's technically correct in that yes the parent shell's visibility is the real controlling factor, but it does express something here that the intent is that this be crate-level. On my projects I have this ignored.
There was a problem hiding this comment.
More than happy to revert these and add a crate-wide exception.
|
A typical way would be to set the priority of the clippy lint group "nursery" to |
4 participants
Fixes for
clippy::nursery, including adding to CI