Add clang-tidy in a GitHub workflow

[gnawme]

This PR is a companion to #4629; per @kunaltyagi, it only adds the GitHub Workflow to enable clang-tidy checks in CI, but does not actually enable clang-tidy in the PCL repo itself.

[kunaltyagi]

Any thoughts on why we don't see Github workflow in the list of checks?

[kunaltyagi]

Apart from lack of github workflow viz, and minor comment, LGTM

Thoughts on adding a docker image for clang-tidy? (env + clang + git)? @larshg @mvieth

[larshg]

Apart from lack of github workflow viz, and minor comment, LGTM

Thoughts on adding a docker image for clang-tidy? (env + clang + git)? @larshg @mvieth

Maybe just run it directly on ubuntu_latest / fixed ubuntu.
https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md

Then its just clang-tidy that needs installed?

It does give potential issues, that MS might change the versions installed, but it would avoid pulling a docker image?

[kunaltyagi]

IIRC, doesn't clang-tidy require all dependencies to be installed?

[larshg]

IIRC, doesn't clang-tidy require all dependencies to be installed?

Ahh, that might be. I'm not that familiar with clang/tidy.

I can see Cmake is run as well, which will fail with missing dependencies.

Is git required explicitly installed? Just realized it used to do the diff.

[gnawme]

Any thoughts on why we don't see Github workflow in the list of checks?

Not sure, it was triggered automatically in the previous PR once I added the workflow files

[kunaltyagi]

I think we need to merge this into master before the github action is activated. We saw something similar with Azure pipelines as well

[gnawme]

I think we need to merge this into master before the github action is activated. We saw something similar with Azure pipelines as well

See reply to your previous comment.

[mvieth]

Closing and re-opening to trigger checks ...

[mvieth]

Please also rebase on the current master branch, otherwise the compile_commands.json is not generated

[gnawme]

@mvieth Opened #5304 to address issues that have crept in since #4629.

Please assign it to me.

EDIT: Or, if you like, I could add the fixes to this PR.

[mvieth]

• the -fix flag is probably also not needed?
• specifying -DCMAKE_EXPORT_COMPILE_COMMANDS=ON here is a good idea. I would then suggest to remove set(CMAKE_EXPORT_COMPILE_COMMANDS ON) from https://github.com/PointCloudLibrary/pcl/blob/master/CMakeLists.txt#L11 again because that is only needed for clang-tidy
• please enable the remaining (non-default) PCL modules apps, benchmarks, examples, simulation, global_tests. See for example here: https://github.com/PointCloudLibrary/pcl/blob/master/.ci/azure-pipelines/build/ubuntu.yaml . I hope there aren't too many clang-tidy issues in those modules, otherwise it might be better to do that in a separate PR

[gnawme]

• please enable the remaining (non-default) PCL modules apps, benchmarks, examples, simulation, global_tests. See for example here: https://github.com/PointCloudLibrary/pcl/blob/master/.ci/azure-pipelines/build/ubuntu.yaml . I hope there aren't too many clang-tidy issues in those modules, otherwise it might be better to do that in a separate PR

I'll try running this in my other PR first, and see what happens...

[mvieth]

@gnawme If you rebase this and all checks pass, then this is ready to merge from my side. We can leave enabling the remaining PCL modules for other pull requests

[mvieth]

Unfortunately, there are still 4 clang-tidy errors left. Let's fix them in this pull request (not a separate one this time)?

[mvieth]

Nice, thanks!