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

rename xdot in python.yaml to python-xdot, becase xdot is already tak… #15259

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

k-okada
Copy link
Contributor

@k-okada k-okada commented Jun 16, 2017

…en by ros package, atleast until indigo http://wiki.ros.org/xdot

rewrited version of #15257

some of rosdep keys are multiply defined, and that sometime confuse users.

rename xdot in python.yaml to python-xdot, becase xdot is already taken by ros package, atleast until indigo http://wiki.ros.org/xdot
change xdot to python-xdot, this will break backword compatibility....

@dirk-thomas
Copy link
Member

This needs to be rebased.

@k-okada
Copy link
Contributor Author

k-okada commented Jun 28, 2017

rebased

@dirk-thomas dirk-thomas requested a review from tfoote June 28, 2017 22:51
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, though before we merge this @k-okada could you announce this and the proposed pocketsphinx change to General on Discourse?

@k-okada
Copy link
Contributor Author

k-okada commented Jun 29, 2017

@tfoote ok, i have posted announcement https://discourse.ros.org/t/proposed-rename-of-rosdep-keys-pocketsphinx-and-xdot/1959/2 , or are you inted to create new post for announce?
I also download all released packages with rosinstall_generator ALL --rosdistro kinetic and rosinstall_generator ALL --rosdistro package and grep-ed to find keys, also googled and created tickets for those who uses renamed keys.

@tfoote
Copy link
Member

tfoote commented Jun 29, 2017

Thanks @k-okada the post and grepping for the keys is about all we can do.

@ros/rosdistro-maintainers all set for merge.

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grepping for the keys is about all we can do

I am not sure if any of you has actually done this but currently at least two released ROS packages use this key. Therefore I don't think this should be merge until after PRs for these packages have been proposed to notify the maintainers. As far as I can see it affects:

@k-okada
Copy link
Contributor Author

k-okada commented Jun 30, 2017

I am not sure if any of you has actually done this

I did and had same results as you did. I have already filed a ticket to thier repository and one of them respond already, so I can take care of them.

srv/srv_tools#16
plasmodic/ecto#297

@k-okada
Copy link
Contributor Author

k-okada commented Jul 7, 2017

@tfoote @dirk-thomas I think I have satisfied your change request, please check again

@dirk-thomas dirk-thomas merged commit 59ee1c9 into ros:master Jul 7, 2017
@k-okada k-okada deleted the fix_duplicates_xdot branch July 7, 2017 15:43
@mikaelarguedas mikaelarguedas mentioned this pull request Apr 20, 2018
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.

3 participants