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

[melodic]: Switch dependency back to python-rosdep. #116

Closed
wants to merge 1 commit into from

Conversation

clalancette
Copy link

Prior to #109 , installing
ros-melodic-desktop on Melodic installed the rosdep command-line.
After that change, it no longer does. For backwards compatibility
reasons, revert that change just for melodic.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

If we don't want to change this dependency back, I'd suggest that the other place to make this change is in https://github.com/ros/metapackages/blob/melodic-devel/ros_core/package.xml . @dirk-thomas thoughts?

Prior to #109 , installing
ros-melodic-desktop on Melodic installed the `rosdep` command-line.
After that change, it no longer does.  For backwards compatibility
reasons, revert that change just for melodic.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette changed the base branch from noetic-devel to melodic-devel March 25, 2020 13:28
@dirk-thomas
Copy link
Member

This package does not use any of the rosdep command line tools and therefore shouldn't depend on it.

The same goes for the metapackage - nothing at runtime requires rosdep to be present so it should not be a dependency.

Instead the instructions should be updated to not rely on transitive dependencies.

@dirk-thomas dirk-thomas deleted the switch-back-to-python-rosdep branch March 25, 2020 14:46
@clalancette
Copy link
Author

Instead the instructions should be updated to not rely on transitive dependencies.

But the thing is, this is a breaking change for Melodic. For Noetic, sure; we can just say from the beginning that you need to install rosdep now. Breaking backwards compatibility like this for a 2 year old distribution just seems like the wrong thing to do here.

@dirk-thomas
Copy link
Member

The only think broken are the instructions. Relying on transitive dependencies is not something which should be done. If dependencies are changed that is exactly what will happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants