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

Remove unnecessary build dependency on std_msgs. #145

Merged
merged 2 commits into from
Oct 4, 2022

Conversation

clalancette
Copy link
Contributor

@clalancette clalancette commented Sep 28, 2022

Instead, make it a test_depend, which is much more appropriate. We also make it so that we don't export the 'std_msgs' dependency to downstream packages, as only our tests depend on it.

While we were in here cleaning things up, make a couple of other minor fixes:

  1. Remove an unnecessary #include to rcutils.
  2. Remove a duplicated call to ament_cmake; that is already a transitive dependency via ament_cmake_ros.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Note that this depends on ros2/rosidl#708 to fully work.

Fixes #144

@clalancette clalancette requested a review from a team as a code owner September 28, 2022 14:40
@clalancette clalancette requested review from Karsten1987 and jhdcs and removed request for a team September 28, 2022 14:40
@clalancette
Copy link
Contributor Author

Full CI, with this PR and ros2/rosidl#708:

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

Instead, make it a test_depend, which is much more appropriate.
We also make it so that we don't export the 'std_msgs' dependency
to downstream packages, as only our tests depend on it.

While we were in here cleaning things up, make a couple of other
minor fixes:

1.  Remove an unnecessary #include to rcutils.
2.  Remove a duplicated call to ament_cmake; that is already
    a transitive dependency via ament_cmake_ros.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
SKIP_GROUP_MEMBERSHIP_CHECK isn't needed when SKIP_INSTALL
is specified, and also we tied skipping dependency export
to SKIP_INSTALL.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette force-pushed the clalancette/remove-std-msgs-dep branch from f743913 to 4a08bc8 Compare October 1, 2022 18:07
Copy link

@jhdcs jhdcs left a comment

Choose a reason for hiding this comment

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

These changes look good to me.

@clalancette
Copy link
Contributor Author

CI:

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

@clalancette
Copy link
Contributor Author

CI is green, and while I could merge this one in now, I'm actually going to wait for ros2/rosidl#708 to be approved before doing that.

@clalancette
Copy link
Contributor Author

All right, the rosidl PR has been merged, so I'll go ahead and merge this one as well.

@clalancette clalancette merged commit 84817ea into rolling Oct 4, 2022
@clalancette clalancette deleted the clalancette/remove-std-msgs-dep branch October 4, 2022 18:06
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.

libstatistics_collector has unnecessary dependency on std_msgs
2 participants