Skip to content

Restore the ability to use Clippy and fix +-50 lints#251

Open
daniel-levin wants to merge 1 commit intooxidecomputer:masterfrom
daniel-levin:clippy
Open

Restore the ability to use Clippy and fix +-50 lints#251
daniel-levin wants to merge 1 commit intooxidecomputer:masterfrom
daniel-levin:clippy

Conversation

@daniel-levin
Copy link
Copy Markdown

Noticed clippy was spewing lints but unable to fix them. For some reason the cmp_owned lint results in non-compileable code. After manually fixing those four locations and integrating @jclulow's suggestions from now-cancelled #249 this PR results.

@daniel-levin
Copy link
Copy Markdown
Author

Filed an issue in Clippy reporting the broken lint: rust-lang/rust-clippy#16897

Copy link
Copy Markdown
Contributor

@citrus-it citrus-it left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Daniel, just a couple of take-them-or-leave-them comments.
Leaving for @jclulow to take a look.

Comment on lines 6 to 8
#
# Report a specific error in the case that the toolchain is too old for
# let-else:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest this comment is removed, or updated to explain what it is in 1.70.0 that we depend on.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call out. I want to bump the MSRV in a subsequent PR, which'll allow us to use edition 2024 and get some nice additional Clippy-driven syntax cleanups.

|| relpath.starts_with("kernel")
|| relpath == PathBuf::from("usr")
|| relpath == PathBuf::from("usr/share")
|| *relpath == *"usr"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about the more natural

relpath == Path::new("usr")

instead of the double deref?

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.

2 participants