Drops legacy launch API usage. #311
Conversation
hidmic
force-pushed
the
hidmic/drop-legacy-launch
branch
2 times, most recently
from
01e50e7
to
3da64a1
Compare
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
hidmic
force-pushed
the
hidmic/drop-legacy-launch
branch
from
3da64a1
to
67c01f8
Compare
hidmic
added
in review
| @@ -28,7 +28,6 @@ | |||
| <test_depend>ament_cmake_pytest</test_depend> | |||
| <test_depend>ament_lint_auto</test_depend> | |||
| <test_depend>ament_lint_common</test_depend> | |||
| <test_depend>launch</test_depend> | |||
There was a problem hiding this comment.
Why was this dependency removed? Python modules from that package are clearly imported in the code below.
There was a problem hiding this comment.
Yes, it clearly still needs that dep.
There was a problem hiding this comment.
Though launch_testing includes launch as an exec_depend. Is that sufficient in this case?
There was a problem hiding this comment.
You never want to rely on transitive dependencies. If you use something directly you should declare it as a dependency. What if launch_testing would be refactored into a standalone package? Then this package would fail.
There was a problem hiding this comment.
Oops, my bad. I think I removed them during a previous refactor, when launch_testing was the only thing needed for testing. I'll get them back ASAP.
Connected to ros2/launch#159.