CI: Add clangd-tidy hook for pre-commit#118162
CI: Add clangd-tidy hook for pre-commit#118162Repiteo wants to merge 1 commit intogodotengine:masterfrom
clangd-tidy hook for pre-commit#118162Conversation
There was a problem hiding this comment.
I personally like this approach of implementing our own customizable pre-commit action.
Regarding the clangd-tidy changes required to make this work, what are you thinking of?
I could put a fork together that has what we want (including --fix)
Ownership would be passed to Godot once ready
| @@ -79,6 +79,18 @@ repos: | |||
|
|
|||
| - repo: local | |||
| hooks: | |||
| # Not automatically triggered (because it requires compile_commands.json to be up-to-date). | |||
There was a problem hiding this comment.
Would it be possible for our now hand-made pre-commit action to run scons compiledb_gen_only to update the compiledb forcefully, thus enabling this stage as automatic?
There was a problem hiding this comment.
I like where your head's at, but it's unfortunately unsuitable for a pre-commit hook because of how long that takes even in isolation. What we might be able to do is a wrapper script which either skips or fails the hook if that file isn't present... Will play around with that today
1cea889 to
5e5d84f
Compare
clangd-tidyas GitHub Action #117654Implements
clangd-tidyas a proper pre-commit hook, making the tool more readily accessible for all users. This required reworking the pre-commit action into our own implementation, which is arguably for the best as that action has been unsupported for years now & is the main source of false-positive warnings in CI outputHowever: despite being fully-functional, I'm hesitant to implement this as-is. As there's no way to enforce
compile_commands.json, nor is there a reliable way to exclude platform-specific files, the hook stage is still "manual". To make this into a readily-accessible hook, I believe that we'd have to either make upstream changes toclangd-tidyor produce our own wrapper script to achieve our desired results. Having said that, I'm setting this up as-is in order to verify that the pre-commit action functions as expected, which will get a dedicated PR if so