Skip to content

[ament_cmake_cppcheck] Fix file exclusion behavior#329

Merged
audrow merged 1 commit intoament:masterfrom
aprotyas:aprotyas/ament_cmake_cppcheck_fix_file_exclusion
Dec 8, 2021
Merged

[ament_cmake_cppcheck] Fix file exclusion behavior#329
audrow merged 1 commit intoament:masterfrom
aprotyas:aprotyas/ament_cmake_cppcheck_fix_file_exclusion

Conversation

@aprotyas
Copy link
Copy Markdown
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
aprotyas commented Oct 17, 2021
View reviewed changes
@aprotyas
Copy link
Copy Markdown
Contributor Author

aprotyas commented Oct 17, 2021

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
Copy Markdown
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
Copy Markdown
Contributor Author

aprotyas commented Oct 19, 2021

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
Copy Markdown
Contributor Author

aprotyas commented Oct 20, 2021

Re-running Windows CI: Windows Build Status

@aprotyas
Copy link
Copy Markdown
Contributor Author

aprotyas commented Oct 27, 2021

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
Copy Markdown
Contributor

audrow commented Nov 19, 2021

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

@aprotyas
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

audrow commented Dec 10, 2021

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

@aprotyas
Copy link
Copy Markdown
Contributor Author

aprotyas commented Dec 11, 2021

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.

2 participants

@aprotyas @audrow