Bug fixes and CI addition

erenon / bazel_clang_tidy

Run clang-tidy on Bazel C++ targets directly, efficiently, with caching enabled

MIT License

109 stars 58 forks source link

Bug fixes and CI addition #6

Closed nathaniel-brough closed 3 years ago

nathaniel-brough commented 3 years ago

Really excited to finally pull this into our build! Great job here.

This change set adds;

Bazel format and linter targets
Fixes bug where rule did not specify the toolchain type that was used but requested a toolchain
Ensures the linters are run in the github actions CI
Adds a dependency injection point so that the .clang-tidy filegroup targets can be overriden from the command line
Adds docs about ^^

I'm happy to break these out into seperate PR's if that is easier.

erenon commented 3 years ago

Wow, thanks! Most of this looks really good. Except one thing: the first commit. IIUC, as BUILD now depends on com_github_bazelbuild_buildtools, users are required to add the boilerplate deps to their WORKSPACE file, and that is too much extra dependency for too little benefit for the user.

So either drop it completely, or move tests into a test/ package, so users will not be affected.

Thanks!

nathaniel-brough commented 3 years ago

Happy to drop it. Though just as a note it's a dev dependency and the user isn't required to include it in their WORKSPACE. It's simply for formatting and linter support.

nathaniel-brough commented 3 years ago

I think I misunderstood what you where saying. Your right, it does make sense to drop that commit.

erenon commented 3 years ago

Thanks!