Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clippy lints #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jayvdb
Copy link

@jayvdb jayvdb commented Nov 29, 2024

fwiw, I am working on a feature, so would like the abundance of preexisting clippy lints to be cleared so I can keep my own code tidy.

I have not fixed three clippy lints that need a little more investigation or discussion. I dont mind a few clippy lints appearing, as I can still easily see any lint problems that are being introduced by me.

@jayvdb jayvdb mentioned this pull request Nov 30, 2024
@kamadak
Copy link
Owner

kamadak commented Dec 2, 2024

Thank you for the patch.
I will merge some parts (e.g., std::u64::MAX --> u64::MAX, which was not used before to support Rust 1.40 and I have now dropped support for that version), but may not merge some parts that I do not like (e.g, .map(|&x| x) --> .copied(), which breaks symmetry with the surrounding code and increase my cognitive load.

Anyway, reducing clippy lints is generally good, so I will try, thanks.

@jayvdb jayvdb force-pushed the clippy-lint-fixes branch from 7445924 to 8034a67 Compare December 2, 2024 19:26
@jayvdb
Copy link
Author

jayvdb commented Dec 2, 2024

I have removed the .copied() replacements that break symmetry, and added a lints.clippy table in Cargo.toml to tell clippy which lints you dont want warnings about.

Feel free to simple put "revert" on other bits of the PR that you dont like, and I will revert them and tell clippy to stop warning about those in future.

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