crates/*: Fix most clippy lints#1551
Conversation
Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Colin Walters <walters@verbum.org>
Signed-off-by: Colin Walters <walters@verbum.org>
There was a problem hiding this comment.
Code Review
This pull request applies a series of fixes suggested by cargo clippy. The changes are mostly stylistic and idiomatic improvements, such as using more concise language features and removing redundant code. The changes look good and improve the overall code quality. I've found one minor opportunity for improvement by removing a redundant .clone() call.
| subtask: layer_type.into(), | ||
| description: format!("{layer_type}: {short_digest}").clone().into(), | ||
| id: format!("{short_digest}").clone().into(), | ||
| id: short_digest.to_string().clone().into(), |
There was a problem hiding this comment.
The .clone() here is redundant. short_digest.to_string() creates a new String, and .into() can consume it directly to create the Cow<'t, str>. You can simplify this line. The same applies to the description field on the line above.
| id: short_digest.to_string().clone().into(), | |
| id: short_digest.to_string().into(), |
Signed-off-by: Colin Walters <walters@verbum.org>
bf4efdf to
adab6b0
Compare
jeckersb
left a comment
There was a problem hiding this comment.
So close to having a bare cargo clippy be clean! Last two things maybe for a followup?
warning: this function has too many arguments (8/7)
--> crates/tmpfiles/src/lib.rs:318:1
|
318 | / fn convert_path_to_tmpfiles_d_recurse<U: uzers::Users, G: uzers::Groups>(
319 | | out_entries: &mut BTreeSet<String>,
320 | | out_unsupported: &mut Vec<PathBuf>,
321 | | users: &U,
... |
326 | | readonly: bool,
327 | | ) -> Result<()> {
| |_______________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
= note: `#[warn(clippy::too_many_arguments)]` on by default
Would be easy enough to refactor this to take a struct of options instead, I agree with clippy that 8 arguments is a bit much.
Checking bootc-sysusers v0.1.0 (/var/home/jeckersb/git/bootc/crates/sysusers)
warning: `bootc-tmpfiles` (lib) generated 1 warning
error: use of a disallowed method `str::len`
--> crates/sysusers/src/lib.rs:132:75
|
132 | let idx = s.find(|c: char| c.is_whitespace()).unwrap_or(s.len());
| ^^^
|
= note: use <str>.as_bytes().len() instead
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
= note: requested on the command line with `-D clippy::disallowed-methods`
error: could not compile `bootc-sysusers` (lib) due to 1 previous error
We explicitly check for this via clippy.toml, should fix. Also an existing bug that the target for validate-rust does not include -D clippy::disallowed-methods which has let this sneak past CI.
I don't have a strong opinion here, but I just allowed it for now.
Done in #1552 |
Just a run of
cargo clippy --fixplus some manual bits.I keep going back and forth on clippy...it's really a spectrum between useful and noisy.