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.

Sign up for GitHub

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

Jump to bottom

[ament_cmake_cppcheck] Fix file exclusion behavior #329

Merged
merged 1 commit into from
Dec 8, 2021
Merged

[ament_cmake_cppcheck] Fix file exclusion behavior #329

merged 1 commit into from
Dec 8, 2021

Conversation

aprotyas
Copy link
Contributor

@aprotyas aprotyas commented Oct 17, 2021 •
edited
Loading

The EXCLUDE argument of the ament_cppcheck CMake function is
a list, i.e. a multi-value keyword. As such, it needs to be placed
out of the one-value keywords from the cmake_parse_arguments
function call.

Without this change, the following invocation:

ament_cppcheck(EXCLUDE foo bar baz)

produces the following CLI command:

ament_cppcheck [...] bar baz --exclude foo

and not the desired CLI command:

ament_cppcheck [...] --exclude foo bar baz

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

@aprotyas
[ament_cmake_cppcheck] Fix file exclusion behavior
6ddee14 
The `EXCLUDE` argument of the `ament_cppcheck` CMake function is
a list, i.e. a multi-value keyword. As such, it needs to be placed
out of the one-value keywords from the `cmake_parse_arguments`
function call.

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

CI with linter tests only (--ctest-args -L linter)

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

@aprotyas aprotyas mentioned this pull request Oct 17, 2021
Merged
audrow
audrow approved these changes Oct 19, 2021
View reviewed changes
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 with green CI.

@aprotyas
Copy link
Contributor Author

Thank you for the review! Running CI again with linter tests only (--ctest-args -L linter)

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

@aprotyas
Copy link
Contributor Author

Re-running Windows CI: Windows Build Status

@aprotyas
Copy link
Contributor Author

The test failure in Windows CI is unrelated and also seen in the Windows nightly: https://ci.ros2.org/view/nightly/job/nightly_win_rel/2099/testReport/

@aprotyas aprotyas mentioned this pull request Nov 18, 2021
Merged
@audrow
Copy link
Contributor

audrow commented Nov 19, 2021

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

@aprotyas
Copy link
Contributor Author

aprotyas commented Nov 27, 2021 •
edited
Loading

I think the linter failures were addressed in ros2/rmw_implementation#200, so I'll just re-run CI:

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

@audrow
Copy link
Contributor

audrow commented Nov 30, 2021

I think the linter failures were addressed in ros2/rmw_implementation#200, so I'll just re-run CI:

Thanks, @aprotyas!

@aprotyas
Copy link
Contributor Author

aprotyas commented Dec 8, 2021

@audrow could you make a new release after merging this please? That would unblock ros2/geometry2#469. Thank you!

@audrow audrow merged commit fd2feb1 into ament:master Dec 8, 2021
@audrow
Copy link
Contributor

audrow commented Dec 8, 2021 •
edited
Loading

@audrow could you make a new release after merging this please? That would unblock ros2/geometry2#469. Thank you!

Sure, I'll try to do it today.

@aprotyas aprotyas deleted the aprotyas/ament_cmake_cppcheck_fix_file_exclusion branch December 8, 2021 18:52
@audrow
Copy link
Contributor

audrow commented Dec 10, 2021

Here's the release for rolling: ros/rosdistro#31443.

@aprotyas
Copy link
Contributor Author

Thanks!

aprotyas added a commit to aprotyas/ament_lint that referenced this pull request Apr 7, 2022
@aprotyas
[ament_cmake_cppcheck] Fix file exclusion behavior (ament#329)
36f8e8b 
The `EXCLUDE` argument of the `ament_cppcheck` CMake function is
a list, i.e. a multi-value keyword. As such, it needs to be placed
out of the one-value keywords from the `cmake_parse_arguments`
function call.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
(cherry picked from commit fd2feb1)
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
This was referenced Apr 7, 2022
Merged
Closed
mjeronimo pushed a commit that referenced this pull request Apr 22, 2022
@aprotyas
[ament_cmake_cppcheck] Fix file exclusion behavior (#329) (#367)
bfdebca 
The `EXCLUDE` argument of the `ament_cppcheck` CMake function is
a list, i.e. a multi-value keyword. As such, it needs to be placed
out of the one-value keywords from the `cmake_parse_arguments`
function call.

Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
(cherry picked from commit fd2feb1)
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
Reviewers

@audrow audrow audrow approved these changes

Assignees

@audrow audrow

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@aprotyas @audrow