search

envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
25.1k stars 4.82k forks source link

Removes unused and unreferenced run_clang_tidy.sh #37320

Closed mathetake closed 3 days ago

mathetake commented 3 days ago

run_clang_tidy.sh has been removed from the CI in https://github.com/envoyproxy/envoy/pull/29848 and not used anymore. Furthermore, it's broken on the main branch, so it's better to remove the file completely to avoid confusing new contributors.

@phlax

mathetake commented 3 days ago

wait, now i am suspicious that clang-tidy is not working on CI at all after https://github.com/envoyproxy/envoy/pull/29848

phlax commented 3 days ago

@mathetake yeah - it was disabled a long time ago for a bunch of reasons - but essentially - as implemented it didnt work - missed more than in caught - created failure on other peoples changes

there is a do_ci.sh target tho - which i run periodically and fix - but i confess i did not for a while

mathetake commented 3 days ago

that sounds bad to be honest, the more we hold on, the more difficult to get it back on track ...

phlax commented 3 days ago

~relatedly i accidentally accepted a dependabot pr to update the clang-tidy bin that it uses - that almost certainly needs to be reverted to the llvm version we are using

mathetake commented 3 days ago

yeah clang-tidy must match the LLVM version we use (currently LLVM 14 )

mathetake commented 3 days ago

anyways can we merge this? no need to maintain this shell anymore regardless of what we are going to do

phlax commented 3 days ago

really up for getting it working - even initially just running on a schedule so we can see and fix any residual issues until its good

the replacement code uses aspects rather than the older way of building ~everything and reusing any artefacts, so theoretically be faster, but with bazel overheads thrown in