This PR improves subtree exclusion by rewriting lint checks to reuse common exclusion logic. Most lint checks previously did not exclude crypto/ctaes, the locale linter did not exclude src/crc32
Makes the include guards lint check print all missing include guards before exiting.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process. A summary of reviews will appear here.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.
Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
Leave a comment here, if you need help tracking down a confusing failure.
Any thoughts on splitting the first commit out into a separate pull? Seems to be a requested feature by three people so far, but I am not sure about bundling it for review with the other changes here.
You are right, split out the first commit into: #30219
This makes all the lint checks written in rust exclude subtrees, and
refactors the exclusion lists into a separate file.
Make the include guard linter print all warnings before exiting, and
reuse common exclusion logic to improve subtree exclusion (crypto/ctaes
was previously not excluded)
Rewrites the 'includes' linter to make use of common exclusion logic,
improving subtree exclusion. (crypto/ctaes was previously not excluded)
This commit rewrites the spelling linter to reuse common
exclusion logic and improves subtree exclusion. ('crypto/ctaes' was
previously not excluded)
This commit rewrites the locale linter to reuse common
exclusion logic and improves subtree exclusion (src/crc32c was
previously not excluded)
Rewrites the python encoding lint check to make use of
common exclusion logic.
183- "./src/",
184- ":(exclude)src/util/fs.h",
185- ])
186+ .args(["grep", "std::filesystem", "--"])
187+ .args(["--", "./src"])
188+ .args(exclude::get_pathspecs_exclude_std_filesystem())
nit in 9d1423a90a3b9748426deec7bafe8b12113e1baf: Not sure about moving a single string literal that is used in a single function to a new module into a new function.
args allows for chaining, so if you want to append more args to the existing ones, my recommendation would be to just call args one more time with the args that you want to append.
I think the lint tests can be easy to understand and pragmatic, and adding too many layers may not be useful to understand them.
(This will also make the diff smaller, and is thus easier to review)
🐙 This pull request conflicts with the target branch and needs rebase.