Deprecate the shared_ptr<MessageT> subscription callback signatures#2975
Deprecate the shared_ptr<MessageT> subscription callback signatures#2975ahcorde merged 6 commits intoros2:rollingfrom
Conversation
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
|
I have pulled and built the ROS2 repos from https://raw.githubusercontent.com/ros2/ros2/rolling/ros2.repos (Please let me know if there are any other repos I should include) Removing
Removing/Deprecating
|
fujitatomoya
left a comment
There was a problem hiding this comment.
@mini-1235 thanks for fixing up all the related packages and repositories.
changing signatures are totally fine things to do including this PR, but i want to discuss with you on either we should enforcement or deprecation here for the next release.
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
|
I am not sure how to deal with the deprecated tests here rclcpp/rclcpp/test/rclcpp/test_any_subscription_callback.cpp Lines 683 to 723 in 1500449 Should I add |
Please do! Then we can remove them in the next cycle with find/replace |
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Added! |
fujitatomoya
left a comment
There was a problem hiding this comment.
lgtm with green CI.
|
Pulls: #2975 |
|
@mini-1235 looks like there were some uncrustify issues here. Can you address those? |
|
As reported in uncrustify/uncrustify#4250, this is a known issue in uncrustify 0.78.1. To avoid timeout, I will skip the uncrustify check here |
Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Description
Fixes #2972
Is this user-facing behavior change?
Yes
Did you use Generative AI?
No
Additional Information
When working on this PR, I noticed that #1713 only deprecates
MessageT. However, looking at the comment here:rclcpp/rclcpp/include/rclcpp/any_subscription_callback.hpp
Lines 140 to 154 in eb49444
it looks like the type_adaptation/serialized message are not yet deprecated, how should we proceed?
In the meantime, I have also updated the tests for type_adaptation/serialized to use
std::shared_ptr<const>