-
Notifications
You must be signed in to change notification settings - Fork 13.3k
bootstrap: allow to set clippy.toml for x.py clippy #137785
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @Mark-Simulacrum. Use |
This PR modifies If appropriate, please update |
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.
r? bootstrap
Maybe someone has stronger opinions about clippy...
let config = LintConfig::new(run.builder); | ||
let config = LintConfig::new( | ||
run.builder, | ||
Some("src/etc/clippy_configs/compiler/clippy.toml".into()), |
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.
This seems like a bit of an odd place to put clippy configs.. maybe we can move them to e.g. src/bootstrap, similar to profile files?
I'm also wondering if there's any clippy-native feature for these configs that we could be using instead? For example package-specific configs or something like that?
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.
Yes, location not really important, i can move it.
Lints can be set via https://doc.rust-lang.org/nightly/cargo/reference/manifest.html#the-lints-section, but not config for them https://doc.rust-lang.org/clippy/lint_configuration.html, as i understand.
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.
Hm, I thought we had user-configurable config for e.g. the cfg lints. Maybe those are special cased though.
// FIXME: this skip merging clippy.toml, | ||
// but maybe there is a better way? |
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.
What does this comment mean?
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.
Function named merge
, but it don't merge 1 field, so i left this as note, better rephrase this?
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 don't we want to merge this field?
First two commits adds ability to set
clippy.toml
forx.py clippy
, to be able to tune clippy lints, and setavoid-breaking-exported-api = false
for bootstrap and compiler as example.3rd commit fixes wrong displayed name for rustdoc - now it will be named
rustdoc
when runningpython x.py clippy librustdoc
, notclippy
.I don't like copypasted paths for lint targets in CI, but can't find better way for it.