Update bond_core to modern cmake. #94
Conversation
In particular, make sure it exports targets so downstream users can use those. While we are here, migrate away from ament_target_dependencies, update to C++17, and move the includes down one directory level (for better workspace overlay support). Signed-off-by: Chris Lalancette <clalancette@gmail.com>
| @@ -36,8 +37,8 @@ endif() | |||
|
|
|||
| ament_python_install_package(${PROJECT_NAME}) | |||
|
|
|||
| ament_export_dependencies(ament_cmake) | |||
| ament_export_include_directories(include) | |||
| ament_export_include_directories(include/${PROJECT_NAME}) | |||
There was a problem hiding this comment.
If you export targets, what purpose is this line?
There was a problem hiding this comment.
It's to support downstream packages that haven't yet switched over to targets. It's a nice way to make sure we can gradually enable targets without forcing all downstream packages to switch at the same time.
|
This seems to work in my testing locally, both with Humble and with Rolling. So going ahead and merging this one, thanks for the reviews. |
|
as a general rule, should any project with |
It's a good question. The ROS 2 core, as of Jazzy, has been (almost) completely converted to use That said, if you are OK with that workaround, it is not a bad idea to get started on the conversion, since we eventually do want to deprecate |
|
@clalancette Will you also backport this change into jazzy? |
|
As it stands, Rolling and Jazzy use the same development branch, so I just did a release today of |
In particular, make sure it exports targets so downstream users can use those.
While we are here, migrate away from ament_target_dependencies, update to C++17, and move the includes down one directory level (for better workspace overlay support).