-
Notifications
You must be signed in to change notification settings - Fork 339
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/${PROJECT_NAME} #548
Conversation
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
a759fab
to
ff42087
Compare
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 generally good to me. The Rpr job should be fixed when ros-visualization/rqt_image_view#62 is released.
CI
Jobs |
👨🌾 It seems this PR caused a build regression in the Can you please take a look @sloretz ? 🙏 |
The Rpr checker was not passing when this was merged and it's still not passing (e.g. #552). |
@ivanpauno A sync to testing has not happened: http://repo.ros2.org/status_page/rolling_default.html?q=ros-rolling-rclcpp . Building has rclcpp 15.0.0, but testing still has 14.1.0. The PR job will pass when the sync to testing happens. |
Thanks for the link! I didn't know how to check that. |
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#365This removesEdit: Added back old-style CMake variables to minimize disruptionament_export_libraries
andament_export_include_directories
as they're redundant with the exported CMake targets.Part of ament/ament_cmake#292
This replaces
ament_target_dependencies()
calls withtarget_link_libraries()
.Requires ros2/rclcpp#1855 for the
rclcpp_components::component
target.