Add --packages-select to ament_clang_tidy #287
Conversation
tylerjw
force-pushed
the
clang_tidy_packages_select
branch
from
cbe4ea8
to
d3e1c5a
Compare
| if len(args.packages_select) == 1: | ||
| args.packages_select = args.packages_select[0].strip().split() |
There was a problem hiding this comment.
Note that the point of these lines is that if someone has a newline-separated string of package names that they would like to use with this argument it will be handled nicely. An example of a command that generates a newline-separated list of packages is colcon list --names-only.
There was a problem hiding this comment.
This is needed because if someone passes a newline-separated escaped string of package names it will be a one length list with the string.
tylerjw
changed the title
tylerjw
force-pushed
the
clang_tidy_packages_select
branch
from
de809aa
to
d786ac6
Compare
Signed-off-by: Tyler Weaver <tylerjw@gmail.com> Add comment explaining handling quoted list of space separated package names Update documentation for ament_clang_tidy Signed-off-by: Tyler Weaver <tylerjw@gmail.com>
tylerjw
force-pushed
the
clang_tidy_packages_select
branch
from
d786ac6
to
2b5b6dd
Compare
| if not compilation_dbs: | ||
| print('No compilation database files found', file=sys.stderr) | ||
| return 1 | ||
|
|
||
| bin_names = [ | ||
| # 'clang-tidy', | ||
| 'clang-tidy-6.0', | ||
| 'clang-tidy-' + args.clang_tidy_version, |
There was a problem hiding this comment.
I would make this a separate change. Supporting other versions of clang-tidy requires enhancements to the parsing of errors and warnings. The regex at
There was a problem hiding this comment.
Ok, I'll split this change out. I should have known better. The reason I did this was that we have standardized on clang-10 tooling for moveit in both ros1 and 2. As far as I can tell this works with clang-tidy-10.
There was a problem hiding this comment.
The stdout output may be fine, but I think for certain kinds of errors and warning returned by clang-tidy-10, the xUnit XML file will contain nonsensical snippets.
Either way, I think adding support for a newer version of clang-tidy needs a closer look.
There was a problem hiding this comment.
I'll split this change out now and do some testing. Maybe I can figure out a straightforward way to support clang-tidy-10. It would make using this much easier as 10 is much newer and therefore more easily available in newer Ubuntu now and in the future (hence why we changed in MoveIt).
tylerjw
force-pushed
the
clang_tidy_packages_select
branch
from
2b5b6dd
to
772dcfd
Compare
tylerjw
changed the title
|
Kind of a follow up of #287, as the --packages-select option only makes sense with a combined compile_commands.json which you only get with this mixin, which aggregates them for you there. Otherwise each package would generate its own compile_commands.json file.
Kind of a follow up of #287, as the --packages-select option only makes sense with a combined compile_commands.json which you only get with this mixin, which aggregates them for you there. Otherwise each package would generate its own compile_commands.json file. Signed-off-by: William Woodall <william@osrfoundation.org>
Kind of a follow up of #287, as the --packages-select option only makes sense with a combined compile_commands.json which you only get with this mixin, which aggregates them for you there. Otherwise each package would generate its own compile_commands.json file. Signed-off-by: William Woodall <william@osrfoundation.org>
Signed-off-by: Tyler Weaver tylerjw@gmail.com
Description
This adds a way to filter based on the package name. This does not add any dependencies and makes this simpler to use for a specific package. The package names are derived from the directory names that compile_commands.json files are found it which is something already done in this script on line 116.
This will make my use case of this much simpler if I select the packages to run this over.
Context
This package's REAMDE contains an example of why I want this feature: https://github.com/tylerjw/moveit_ci_tools