ci: fix clang-tidy #10496
ci: fix clang-tidy #10496
Conversation
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
test/integration/fake_upstream.cc
Outdated
| @@ -83,10 +83,10 @@ void FakeStream::encode100ContinueHeaders(const Http::ResponseHeaderMap& headers | |||
| void FakeStream::encodeHeaders(const Http::HeaderMap& headers, bool end_stream) { | |||
| std::shared_ptr<Http::ResponseHeaderMap> headers_copy( | |||
| Http::createHeaderMap<Http::ResponseHeaderMapImpl>(headers)); | |||
| if (add_served_by_header_) { | |||
| if (add_served_by_header_) | |||
There was a problem hiding this comment.
What is this style change? I thought we had mandatory braces for single line if. I think Heartbleed or similar was caused by not doing this.
There was a problem hiding this comment.
No, I'm trying to prove clang-tidy will fail on this, and revert later.
| @@ -1,19 +1,4 @@ | |||
| Checks: 'abseil-*, | |||
| bugprone-*, | |||
| clang-analyzer-*, | |||
There was a problem hiding this comment.
Are we regressing anything by eliminating these?
There was a problem hiding this comment.
Those are checked but not enforced before but I don't think anyone is looking at it. So just check what we are enforcing. Then all errors are written to the fix yaml file so we can check in bash easily.
There was a problem hiding this comment.
Once we get this fixed is it possible to do a complete master run and fix all the regressions? I'm sure there are a lot. Thanks for doing this. Very excited to get this fixed!
|
Need envoyproxy/envoy-build-tools#39 and a image update for export-fixes |
| if [[ -s "${FIX_YAML}" ]]; then | ||
| echo "clang-tidy check failed, potentially fixed by clang-apply-replacements:" | ||
| cat ${FIX_YAML} | ||
| exit 1 |
There was a problem hiding this comment.
nice workaround to the non-informative exit code
|
If you need 2 more test cases, these should be updated to |
|
FYI - I don't mind doing the busy work of fixing the things flagged by clang-tidy once we have this correctly reporting the problems. I'll merge Lizan's branch locally and have a separate PR fixing the issues that we can merge, and that would make this PR green to merge. |
ci/run_clang_tidy.sh
Outdated
| @@ -60,17 +61,27 @@ if [[ "${RUN_FULL_CLANG_TIDY}" == 1 ]]; then | |||
| "${LLVM_PREFIX}/share/clang/run-clang-tidy.py" \ | |||
|
I'm waiting for image update #10506 to make this CI green |
oh yeah - it only runs on the diff. |
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
Signed-off-by: Lizan Zhou <[email protected]>
|
@htuch good to go, verified clang-tidy CI detects failure: https://github.com/envoyproxy/envoy/pull/10496/checks?check_run_id=534880325 |
Description:
Fixes #9512.
Risk Level:
Testing:
Docs Changes:
Release Notes:
Signed-off-by: Lizan Zhou [email protected]