-
Notifications
You must be signed in to change notification settings - Fork 108
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
Add deprecation messages to all messages now in example_interfaces #116
Conversation
Followup to #89 Signed-off-by: Tully Foote <tfoote@osrfoundation.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder about the rationale behind deprecating what's in std_msgs
and moving it to example_msgs
. It's been like this in ROS 1 for over a decade, why the change?
std_msgs/msg/UInt8MultiArray.msg
Outdated
@@ -1,3 +1,8 @@ | |||
# This was originally provided as an example message. | |||
# It is deprecated as of Foxy, it is recommended to create your own |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tfoote consider
# It is deprecated as of Foxy, it is recommended to create your own | |
# It is deprecated as of Foxy. | |
# It is recommended that you create your own |
same elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks that reads better, updated.
It's been in ROS 1 that we generally recommend against using almost any message in std_msgs except Header for a long time for stability.
However from the naming it leads to a lot of confusion.(For example) And we want to take the opportunity in ROS 2 to avoid that ambiguity. The earlier we do it in this cycle the easier and less disruption it will cause. |
Signed-off-by: Tully Foote <tfoote@osrfoundation.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Though their usage has been discouraged for a long time now, it might be worth highlighting that we have "officially" deprecated them in the Foxy release notes.
I guess we shouldn't have run CI for this change since it caused a test failure downstream: https://ci.ros2.org/view/nightly/job/nightly_linux_release/1548/testReport/ros2interface.src.ros2.ros2cli.ros2interface.test/test_cli/test_cli/ See a fix in ros2/ros2cli#516 |
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/soss-a-whole-new-approach-to-your-ros-1-ros-2-bridge/17040/18 |
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/discussion-standardizing-ros-interfaces/33693/1 |
Followup to #89