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

Fix file exclusion behavior in ament_cppcheck and ament_cpplint #299

Merged
merged 3 commits into from
Feb 22, 2021

Conversation

mm318
Copy link
Member

@mm318 mm318 commented Feb 14, 2021

Closes #295

This pull request fixes the issue described at #234 (comment) and possibly replaces #238.

This pull request also makes the behavior between ament_cppcheck and ament_cpplint consistent.

Signed-off-by: Miaofei <miaofei@amazon.com>
Signed-off-by: Miaofei <miaofei@amazon.com>
@mm318
Copy link
Member Author

mm318 commented Feb 14, 2021

CI run with build --packages-up-to ament_cmake_cpplint ament_cmake_cppcheck and test --packages-select ament_cmake_cpplint ament_cmake_cppcheck ament_cpplint ament_cppcheck, because it seems like there's currently no core packages (under ros2.repos) that is using the ament_cpplint(... EXCLUDE ...) and ament_cppcheck(... EXCLUDE ...) features.

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@mm318 mm318 requested a review from ahcorde February 17, 2021 08:53
@mm318 mm318 self-assigned this Feb 17, 2021
@mm318 mm318 requested a review from audrow February 19, 2021 17:59
Copy link
Contributor

@audrow audrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Signed-off-by: Miaofei <miaofei@amazon.com>
@mm318
Copy link
Member Author

mm318 commented Feb 22, 2021

Rerunning CI after fix

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@mm318 mm318 merged commit d17140b into ament:master Feb 22, 2021
aprotyas added a commit to aprotyas/ament_lint that referenced this pull request Oct 17, 2021
This commit fixes the faulty file exclusion behavior reported in
ament#326.

Specifically, it walks down the path where `ament_copyright` has
been invoked to build a map of files to be tested, respecting
the exclusion list by globbing and matching against files in the
tree.

Changes inspired by ament#299.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
aprotyas added a commit to aprotyas/ament_lint that referenced this pull request Oct 17, 2021
This commit fixes the faulty file exclusion behavior reported in
ament#326.

Specifically, the exclusion list is matched against traversed
files in the `crawler` module.

Changes inspired by ament#299.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
kenji-miyake pushed a commit to kenji-miyake/ament_lint that referenced this pull request Oct 20, 2021
…t#299)

* fix exclude behavior in ament_cppcheck and ament_cpplint

Signed-off-by: Miaofei <miaofei@amazon.com>

* fix flake8 errors

Signed-off-by: Miaofei <miaofei@amazon.com>

* add missing realpath() conversion

Signed-off-by: Miaofei <miaofei@amazon.com>
audrow pushed a commit that referenced this pull request Nov 4, 2021
* [ament_copyright] Fix file exclusion behavior

This commit fixes the faulty file exclusion behavior reported in
#326.

Specifically, the exclusion list is matched against traversed
files in the `crawler` module.

Changes inspired by #299.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>

* Update excluded file path in copyright tests

Since file names are not indiscriminately matched throughout the
search tree anymore, the excluded files listed in the copyright
tests need to be updated relative to the root of the package.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>

* Add test cases to check exclusion behavior

Specifically, these tests check for:

- Incorrect exclusion of single filenames.
- Correct exclusion of relatively/absolutely addressed filenames.
- Correct exclusion of wildcarded paths.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>

* Add unit tests for crawler module

These unit tests make sure both search and exclusion behaviors are
correctly demonstrated by the `ament_copyright.crawler` module.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ament_cpplint doesn't support EXCLUDE parameter
2 participants