search

ros2 / demos

Apache License 2.0
494 stars 329 forks source link

Remove action_tutorials_interfaces. #701

Closed clalancette closed 1 week ago

clalancette commented 2 weeks ago

We have this message duplicated three times in the core; once in here, once in example_interfaces, and once in test_interface_files. That seems a bit excessive, so remove this third copy and make the action_tutorials use the example_interfaces version of Fibonacci.action instead.

This change obviously changes the dependencies of the action_tutorials_cpp and action_tutorials_py packages, in that it removes action_tutorials_interfaces and adds in example_interfaces. I believe that this should be fine from an inter-repository standpoint, as the demos repository already contains other packages (composition, quality_of_service_demo_cpp, demo_nodes_py, demo_nodes_cpp) that depend on example_interfaces.

This will fix #702 by at least removing one of our duplicate copies of this message.

clalancette commented 2 weeks ago

Pulls: ros2/demos#701 Gist: https://gist.githubusercontent.com/clalancette/f40892f15019e2125c75b9041490e888/raw/9eee2c252a0399a1fc03246677ea5fcd590e76fd/ros2.repos BUILD args: --packages-above-and-dependencies action_tutorials_py action_tutorials_cpp TEST args: --packages-above action_tutorials_py action_tutorials_cpp ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14488

clalancette commented 2 weeks ago

so the expectation is that nobody in the downstream depends on this action-tutorials-interfaces, i believe that should be fine.

Actually, this is a great question, and one I should have considered.

I took a look at all of the packages currently released into Rolling, and besides action_tutorials_py and action_tutorials_cpp (fixed here), the following depend on action_tutorials_interfaces:

I'll follow up with those projects to remove their use of action_tutorials_interfaces before we merge this in.

ahcorde commented 2 weeks ago

PRs created

clalancette commented 1 week ago

I'm going to go ahead and merge this one in. That will break simple_actions for now, but it should be easy enough to fix it later on just by merging the above PR. Thanks @ahcorde for the extra work here!