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 unused ament_cmake dependency. #343

Merged
merged 1 commit into from
May 21, 2021

Conversation

nuclearsandwich
Copy link
Contributor

REP-136 recommends that third-party (non-ROS) packages add a runtime
dependency on catkin so that the setup scripts provided by the catkin
package which set up a ROS environment are installed along with that
package. In ROS 2, those scripts are provided by the ros_workspace
package and bloom transparently injects a dependency on ros_workspace
into every released package.

It would be possible to use ament_cmake (or more likely the individual
ament_cmake_core package to optionally provide the package.xml and
ament resource index registration but since what you have is working
fine I think just removing the unecessary dependency is a good next
step. As mentioned in
flexible-collision-library/fcl#536 (comment)
we haven't produced a definitive recommendation for third party packages
in ROS 2 but I've added it to a meeting agenda and we'll document the
conclusions of that discussion and share them as well.

Thanks @wxmerkt for prompting this in flexible-collision-library/fcl#536 (comment)

REP-136 recommends that third-party (non-ROS) packages add a runtime
dependency on catkin so that the setup scripts provided by the catkin
package which set up a ROS environment are installed along with that
package. In ROS 2, those scripts are provided by the ros_workspace
package and bloom transparently injects a dependency on ros_workspace
into every released package.

It would be possible to use ament_cmake (or more likely the individual
ament_cmake_core package to optionally provide the package.xml and
ament resource index registration but since what you have is working
fine I think just removing the unecessary dependency is a good next
step. As mentioned in
flexible-collision-library/fcl#536 (comment)
we haven't produced a definitive recommendation for third party packages
in ROS 2 but I've added it to a meeting agenda and we'll document the
conclusions of that discussion and share them as well.
@wxmerkt
Copy link
Member

wxmerkt commented May 20, 2021

@nuclearsandwich Thank you very much for the insights, very clear explanation, and fix! :-)

@ahornung ahornung merged commit 136c38e into OctoMap:devel May 21, 2021
@ahornung
Copy link
Member

Does this need an urgent release out, or is it just a minor fix that can be included in the next release?

@wxmerkt
Copy link
Member

wxmerkt commented May 21, 2021

It does not require a release right now - we'll include it in the next one - thanks for checking :-).

@nuclearsandwich nuclearsandwich deleted the ros2-drop-ament-cmake-dep branch July 1, 2021 20:56
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

3 participants