Skip to content

Conversation

@nnethercote
Copy link
Contributor

Clippy is a useful tool. This PR fixes/silences all the warnings it finds, and enables it in CI.

Signed-off-by: Nicholas Nethercote <[email protected]>
Signed-off-by: Nicholas Nethercote <[email protected]>
This use of `map` is *very* non-idiomatic.

Signed-off-by: Nicholas Nethercote <[email protected]>
Note: the MSRV for this is 1.73.

Signed-off-by: Nicholas Nethercote <[email protected]>
@nnethercote
Copy link
Contributor Author

Using clippy is very common. It has a lot of lints and not all of them are equally useful or important, so customizing the setup to ignore lints you don't care about is certainly an option, but I haven't done that here.

My first contribution here, so let me know if I've done anything wrong, and I'm happy to adjust things as necessary.

@nnethercote
Copy link
Contributor Author

Info on individual lints is here: https://rust-lang.github.io/rust-clippy/master/index.html

@jk-ozlabs
Copy link
Member

I certainly had a todo for clippifying the tree, thanks for the series. Will take a look through shortly.

@jk-ozlabs
Copy link
Member

Looks like we need a reformat as part of 144710b. Otherwise all looks good!

@jk-ozlabs
Copy link
Member

(and no issues with the MSRV, we don't have/need any specific guarantees there)

@mkj
Copy link
Member

mkj commented Jul 23, 2025

Looks like we need a reformat as part of 144710b. Otherwise all looks good!

Ah, that's the problem where on embedded platforms it's optionally using defmt::info!() which doesn't support named parameters (one day!). But clippy assumes its log::info!().

@mkj
Copy link
Member

mkj commented Jul 23, 2025

optionally using defmt::info!()

Sorry ignore that, those parts weren't touched in this PR.

@nnethercote
Copy link
Contributor Author

I fixed the fmt error.

@jk-ozlabs
Copy link
Member

hm, may also need a component add clippy. @mkj : any issues with this staying in ci.sh, rather than handling the rust env in the github workflow?

@mkj
Copy link
Member

mkj commented Jul 23, 2025

hm, may also need a component add clippy. @mkj : any issues with this staying in ci.sh, rather than handling the rust env in the github workflow?

ci.sh should work fine, the same as the existing for rustfmt. Or in rust-toolchain.toml. I thought both of them were in the default rustup set though, https://ehuss.github.io/rustup/concepts/profiles.html

Signed-off-by: Nicholas Nethercote <[email protected]>
Having no default method seems fine here.

Signed-off-by: Nicholas Nethercote <[email protected]>
Signed-off-by: Nicholas Nethercote <[email protected]>
@nnethercote
Copy link
Contributor Author

There was a needless_lifetime warning on CI that I wasn't seeing locally. CI is running on thumbv7em-none-eabihf and my local machine is x86_64-unknown-linux-gnu, though I don't know why that would cause a difference. Weird.

@mkj mkj merged commit 39e2aae into CodeConstruct:main Jul 24, 2025
3 checks passed
@mkj
Copy link
Member

mkj commented Jul 24, 2025

Thanks!

Looks like the needless_lifetime failure before was during the x86_64-unknown-linux-gnu build, I guess it was just some other environment difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants