Skip to content

Only use --verify-config to check config #94

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

Merged
merged 1 commit into from
Jul 23, 2025

Conversation

mattlangford
Copy link
Contributor

@mattlangford mattlangford commented Jul 20, 2025

This is very similar to #93, but does the same move in the actual clang tidy script.

I verified that this let clang tidy run in a private repo (where before it would return no errors). It also errors out when there are config errors

@mattlangford mattlangford force-pushed the master branch 2 times, most recently from 13c9511 to da52744 Compare July 20, 2025 05:54
@erenon erenon merged commit 7a5d959 into erenon:master Jul 23, 2025
1 check passed
@erenon
Copy link
Owner

erenon commented Jul 23, 2025

Thanks!

@dotnwat
Copy link

dotnwat commented Jul 24, 2025

@mattlangford

I observe that clang-tidy returns 0 success code even when there are verification errors. This is using clang-20.

So we probably should check for the success output "No config errors found." (or fix clang-tidy). Otherwise, config verification failure still just passes through as success it seems.

$ bazel-out/k8-fastbuild/bin/external/toolchains_llvm++llvm+current_llvm_toolchain/clang-tidy --quiet --verify-config; echo $?
/home/nwatkins/src/redpanda/.clang-tidy:14:5: error: Unexpected token in Key Value.
    asdf
    ^
Error parsing /home/nwatkins/src/redpanda/.clang-tidy: Invalid argument
/home/nwatkins/src/redpanda/.clang-tidy:14:5: error: Unexpected token in Key Value.
    asdf
    ^
Error parsing /home/nwatkins/src/redpanda/.clang-tidy: Invalid argument
No config errors detected.
0

@mattlangford
Copy link
Contributor Author

Good catch @dotnwat, the changes in #95 seem to help with that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants