ros-infrastructure / catkin_pkg

Standalone Python library for the catkin build system.
https://github.com/ros/catkin
Other
47 stars 91 forks source link

Added explanation as to why targets should be prefixed with package name #132

Closed davetcoleman closed 8 years ago

davetcoleman commented 8 years ago

Based on discussion here: https://github.com/catkin/catkin_tools/issues/236

dirk-thomas commented 8 years ago

Thanks. LGTM.

NikolausDemmel commented 8 years ago

+1

gavanderhoorn commented 8 years ago

Might be nice to add that you can use set_target_properties like this:

set_target_properties(${PROJECT_NAME}_node PROPERTIES OUTPUT_NAME node PREFIX "")

to remove the prefix from your binary names again. It's not needed as the package name will provide a scope when used with other ROS infrastructure.

davetcoleman commented 8 years ago

@gavanderhoorn that's a good point but I was thinking I didn't want to over-complicate the CMakeLists.txt even further. But thinking about it more, that does seem like a good idea.

gavanderhoorn commented 8 years ago

A valid point, but if we don't add the line, we'll have new users blindly copy/pasting this and end up with things like rosrun someones_pkg someones_pkg_their_node. Not very desirable I'd say.

davetcoleman commented 8 years ago

I've added documentation for this new feature

dirk-thomas commented 8 years ago

Thank you for iterating on this. I have changed the term namespace to prefix. The modified commit has been cherry-picked in 8574d5b9fabf3ba176ea21bc3629f15912e11fc1

davetcoleman commented 8 years ago

thanks!