-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix low-count lints in all crates #7716
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
Conversation
bc5fd3d to
0e600ba
Compare
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
9a12867 to
de38959
Compare
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
Running this command showed a list of all lints across all crates:
```shell
cargo clippy --all-targets --workspace --message-format=json --quiet | jq -r '.message.code.code | select(. != null and startswith("clippy::"))' | sort | uniq -c | sort -h -r
```
This resulted in a list that I added to the `[workspace.lints.clippy]` in the root Cargo.toml. Afterwards, I commented out a few simpler lints. Subsequentely, I will go through this list, trying to address items in smaller batches.
|
GNU testsuite comparison: |
| all = { level = "warn", priority = -1 } | ||
| cargo = { level = "warn", priority = -1 } | ||
| pedantic = { level = "warn", priority = -1 } | ||
| cargo_common_metadata = "allow" # 3240 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why keeping these numbers? They will be outdated tomorrow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sylvestre I use these numbers to guide which one I fix next - at which point I remove the entire line, not just the comment
| })); | ||
| } | ||
| #[cfg(target_os = "linux")] | ||
| #[cfg(not(target_os = "linux"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it changed the behavior, no ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sylvestre correct - I am pretty certain this was a bug in the code. Note that the code block is actually a noop, so it didn't show up in any testing, but this change makes sure that when code is added, it will be correctly compiled
Running this command showed a list of all lints across all crates:
This resulted in a list that I added to the
[workspace.lints.clippy]in the root Cargo.toml. Afterwards, I commented out a few simpler lints. Subsequentely, I will go through this list, trying to address items in smaller batches.Notes
write!instead of creating temp stringsdd.rs- the non-linux cfg was incorrectly used. It was still a noop, so non-consequential