Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filtering to ament_clang_tidy #280

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from
Open

Commits on Jan 4, 2021

  1. Add filtering to ament_clang_tidy

    This change adds two types of filtering to which files are tested with clang-tidy.  The motivation behind this is clang-tidy itself can take a really long time to execute and as a result, we'd like to limit the number of files it tests to those changed in a given PR for CI for moveit.
    
    This adds three arguments to ament_clang_tidy:
    
    The first one allows filtering the packages tested to a single repo.  This is useful in CI to limit the tests to the given repo you are executing on.  Also, if it finds a .clang-tidy config file in the root directory of the repo it will use that instead of the default clang-tidy config.
    **known issue**: the implementation of this depends on on  `catkin_topological_order`.  Is there an ament/ros2 friendly tool for doing the same thing?
    
    This one uses git to compare the current state of the repo with some previous version and test only the files changed.  This enables us to use clang-tidy in ci with large projects like moveit because it limits the scope of the clang-tidy test to the packages that were built.  This depends on the `filter-repo-path` feature above because the git commands used to figure out what files changed need to be run from the repo directory.
    
    I feel the least strong about this feature.  I added this because I wanted it while debugging what clang-tidy command was actually being executed.  I think this could be a useful argument in general for python scripts that execute external programs.
    
    I'm working to improve the ci used by moveit and moveit2.  I wrote this change to help with the migration away from travis we are going through with moveit (hence the catkin dependency).  Because this script is not easily available in ros1 (asfaik) I have this PR to submit the contents of it with this change into a tools directory in moveit for our own use (moveit/moveit#2470).  I'd appreciate advice if there is a better approach I should use to get ament scripts that are useful on ros1 projects.
    
    Signed-off-by: Tyler Weaver <[email protected]>
    tylerjw committed Jan 4, 2021
    Configuration menu
    Copy the full SHA
    3150647 View commit details
    Browse the repository at this point in the history