Skip to content

Conversation

klensy
Copy link
Contributor

@klensy klensy commented Feb 28, 2025

First two commits adds ability to set clippy.toml for x.py clippy, to be able to tune clippy lints, and set avoid-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 running python x.py clippy librustdoc, not clippy.

I don't like copypasted paths for lint targets in CI, but can't find better way for it.

@rustbot
Copy link
Collaborator

rustbot commented Feb 28, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 28, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 28, 2025

This PR modifies src/bootstrap/src/core/config.

If appropriate, please update CONFIG_CHANGE_HISTORY in src/bootstrap/src/utils/change_tracker.rs.

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a 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()),
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

@rustbot rustbot assigned clubby789 and unassigned Mark-Simulacrum Mar 1, 2025
Comment on lines +122 to +123
// FIXME: this skip merging clippy.toml,
// but maybe there is a better way?
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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?

@bors
Copy link
Collaborator

bors commented Jun 26, 2025

☔ The latest upstream changes (presumably #142581) made this pull request unmergeable. Please resolve the merge conflicts.

@clubby789
Copy link
Contributor

clubby789 commented Jun 26, 2025

@rustbot author

(due to the outstanding comment and conflict)

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 26, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 26, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants