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

[ament_cmake_uncrustify] Add file exclude support #330

Merged

Conversation

aprotyas
Copy link
Contributor

In the ament_uncrustify CMake function, the optional list
argument EXCLUDE can now be used as an exclusion specifier.

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

In the `ament_uncrustify` CMake function, the optional list
argument `EXCLUDE` can now be used as an exclusion specifier.

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

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

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

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

Looks like that is being discussed in ros2/launch_ros#273

@audrow
Copy link
Contributor

audrow commented Nov 18, 2021

Hey @aprotyas, I believe that I asked this elsewhere, but don't remember the answer, do you have the ability to merge this, or would you like me to?

@aprotyas
Copy link
Contributor Author

@audrow I don't have merge access in this repository, could you please merge this?

Also, #329 and #328 are ready to land as well - these are all essentially the same changes.

@audrow
Copy link
Contributor

audrow commented Nov 18, 2021

Rerunning CI, just to see if anything has changed:

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

@audrow audrow merged commit 1a3c7b3 into ament:master Nov 19, 2021
@audrow
Copy link
Contributor

audrow commented Nov 19, 2021

Thanks for the PR, @aprotyas!

@aprotyas aprotyas deleted the aprotyas/ament_cmake_uncrustify_add_exclusion branch November 30, 2021 22:57
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.

None yet

2 participants