-
Notifications
You must be signed in to change notification settings - Fork 291
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
Install includes to include/ and misc CMake fixes #225
Install includes to include/ and misc CMake fixes #225
Conversation
There was a problem hiding this 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 with green CI.
@ros-pull-request-builder retest this please |
We're actually going to have to split this up into several different branches, since we only want to land this change on |
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
5348737
to
e4ff192
Compare
I've rebased this onto the latest, which should 🤞 make CI pass here now. |
Make sure to really declare and find all dependencies. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@sloretz I just pushed 0d2b8e5 here, which adds in some additional infrastructure that the CMake and package.xml files were missing. This now looks good to me, and passes CI, so I'm going to approve it. However, since I did enough additional work here, I'd appreciate a comment from you on whether you agree with the changes I've done before I merge this. |
I'm going to go ahead with this one so we can get a release out for Rolling and try to fix the spacenav regression. If anyone wants to do a review after merge, I'm happy to make follow-up changes. |
Part of ros2/ros2#1150
This installs includes to
include/${PROJECT_NAME}
to mitigate include directory search order issues when overriding packages in desktop.Part of ament/ament_cmake#292
This replaces
ament_target_dependencies()
calls withtarget_link_libraries()
.I also made changes to use modern CMake targets, notably by creating IMPORTED targets
wiimote::bluetooth
andwiimote::cwid
to make thewiimote::wiimote_lib
target exportable.Requires ros2/rclcpp#1855 for the
rclcpp_components::component
target.