-
Notifications
You must be signed in to change notification settings - Fork 134
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add note in ExecuteKnownTrajectory service to recommend ExecuteTrajectory action. #29
Conversation
3e6e832
to
2807dea
Compare
Apparently I was wrong; We still want both moveit/moveit#60 (comment) So instead of removing, I added a notice in |
-1 |
Closing, due to @v4hn's comment. Deprecation message is included in the main code. |
Good point for the checksum change. I'm just not sure if updating ONLY comment would change the md5sum, so I asked a question for confirmation (I included there a quick test result I locally did that didn't change checksum). |
There seems to be a wiki page that says comment are excluded for md5 generation. As a non-expert MoveIt! coder, the description in msg/srv/action files is very helpful. There's also an open issue #2 for in-file description enhancement. So reopening. |
Thanks for researching this! Looks like it was a myth - or maybe they changed it at some point? I would prefer to formulate the comment a bit more strict. How about
? |
…ed ExecuteTrajectory action.
2807dea
to
1df4f62
Compare
Ok, commit updated. |
+1 Looks good to me. |
Cherrypicked to jade-devel 40b691e |
Not entirely sure if this suggestion is the right move, but looking at places like this comment we want to switch to the new action.