chore(style): sync submodule exclusion list between tidy and rustfmt #132524
Conversation
As asked in the FIXME comments
rustbot
added
A-meta
| @@ -7,7 +7,6 @@ use ignore::DirEntry; | |||
|
|
|||
| /// The default directory filter. | |||
| pub fn filter_dirs(path: &Path) -> bool { | |||
| // FIXME: sync submodule exclusion list with rustfmt.toml | |||
There was a problem hiding this comment.
Discussion: @onur-ozkan should tidy be reading from rustfmt.toml or we somehow make sure that there's a single source of truth for this? (not for this PR)
There was a problem hiding this comment.
Skip lists for rustfmt and tidy seem not overlapping but if tidy needs to include the skip list of rustfmt, I wanna work on it.
There was a problem hiding this comment.
My other concern is that highlighted rows in the screenshot above (from rustfmt.toml) are not submodules. Maybe some time ago, they were and added to that list. But now they are managed inside the rust repo and rustfmt doesn't format them. I've tried to run x fmt for them and found a lot of errors. Shouldn't they be formatted like other files in the rust repo as well?
There was a problem hiding this comment.
Some of them are not submodules but subtrees and managed by josh, so I don't think this is easily sync'd. Asking Onur because I might be missing context for these particular lists.
There was a problem hiding this comment.
IIRC we don't want tidy to rely on a toml parser because it needs to build fast, so probably not sync'd easily.
There was a problem hiding this comment.
So how about relying on .gitmodules file for both rustfmt and tidy?
|
But anyway, if we do want to sync it should be a follow-up, this PR itself is fine AFAICT. Thanks! @bors r+ rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#132259 (rustc_codegen_llvm: Add a new 'pc' option to branch-protection) - rust-lang#132409 (CI: switch 7 linux jobs to free runners) - rust-lang#132498 (Suggest fixing typos and let bindings at the same time) - rust-lang#132524 (chore(style): sync submodule exclusion list between tidy and rustfmt) - rust-lang#132567 (Properly suggest `E::assoc` when we encounter `E::Variant::assoc`) - rust-lang#132571 (add const_eval_select macro to reduce redundancy) - rust-lang#132637 (Do not filter empty lint passes & re-do CTFE pass) - rust-lang#132642 (Add documentation on `ast::Attribute`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#132524 - ismailarilik:chore/style/sync-submodule-exclusion-list-between-tidy-and-rustfmt, r=jieyouxu chore(style): sync submodule exclusion list between tidy and rustfmt As asked in the FIXME comments
ismailarilik
deleted the
chore/style/sync-submodule-exclusion-list-between-tidy-and-rustfmt
branch
ismailarilik
restored the
chore/style/sync-submodule-exclusion-list-between-tidy-and-rustfmt
branch
ismailarilik
deleted the
chore/style/sync-submodule-exclusion-list-between-tidy-and-rustfmt
branch
As asked in the FIXME comments