Improve clang-tidy check

rhaschke commented 2 years ago

Partially fixes #796:

Don't -export-fixes by default
Use run-clang-tidy instead of clang-tidy to (hopefully) better handle parallel changes to the same files Unfortunately, this changes cmdline API, i.e. options have different names!

TODO: Handle fixes from multiple packages:

[x] parse clang_tidy_args for -export-fixes <filename>, replace filename with some temporary file
[x] run-clang-tidy, writing fixes to this temporary file
[x] merge yaml files into the final one: https://stackoverflow.com/questions/25630633/merging-yaml-config-files-recursively-with-bash https://mikefarah.gitbook.io/yq/v/v3.x/commands/merge

There is an interesting github action, processing clang-tidy's fixes into PR comments: https://github.com/marketplace/actions/pull-request-comments-from-clang-tidy-reports

I triggered a test build for MoveIt: https://github.com/ros-planning/moveit/pull/3250

@EzraBrooks, do you have resources to contribute?

EzraBrooks commented 2 years ago

I'd love to help, but we're currently swamped here with customer deliverables and conference demos. It's that time of the (fiscal) year! Thanks for your work on this.

rhaschke commented 2 years ago

I finally found some time to finish this. Here is an example output. Note, the last commit is needed for proper links on github.

EzraBrooks commented 2 years ago

This is awesome! I'm going to make a PR on my project to use this branch of ICI and test it out.

EzraBrooks commented 2 years ago

Works like a charm!

tylerjw commented 2 years ago

@mathias-luedtke we are using this, and it works well for us. Would you mind reviewing this?

rhaschke commented 1 year ago

Closing and reopening to re-trigger CI. The failure was unrelated to this PR. @mathias-luedtke, is there any other reason holding this back? This PR is not just cosmetic, but actually fixes a bug with clang-tidy generated fixes: The old approach, running multiple tidy instances that fix the code in parallel, caused conflicts.

EzraBrooks commented 1 year ago

This has remained stable in my team's CI and has been excellent in conjunction with this action that highlights our clang-tidy violations in our PR diffs.

130s commented 1 year ago

@mathias-luedtke Would you mind reviewing this?

ros-industrial / industrial_ci

Improve clang-tidy check #798