ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
563 stars 427 forks source link

Fix NodeOptions assignment operator #2656

Closed rdesille closed 1 month ago

rdesille commented 1 month ago

While working with NodeOptions, I realized "enable_logger_service" was copied with the assignment operator. Here is a little PR to fix the issue. I also added some additional checks for primitives members in TEST(TestNodeOptions, copy).

Let me know what you think

christophebedard commented 1 month ago

Nice catch! Looks like this was missed in #2122. I just have one simple comment.

Then we should backport this to Jazzy and Iron.

rdesille commented 1 month ago

I added more tests and a small comment to list attributes that were not / couldn't be tested. Formatting should also be fixed. Let me know what you think.

How does backports to Jazzy and Iron works? Should I open PR to the said branches after this one is approved? Sorry I am new to ROS contributions. :angel:

alsora commented 1 month ago

@rdesille thank you for your contribution!

How does backports to Jazzy and Iron works? Should I open PR to the said branches after this one is approved? Sorry I am new to ROS contributions

We should first finish to review and merge this PR. Then we can try to automatically backport it.

CI run:

christophebedard commented 1 month ago

Pulls: ros2/rclcpp#2656 Gist: https://gist.githubusercontent.com/christophebedard/1de00d2d06f7b1dc7112ca903630b017/raw/30532634394c0b20a75069ce01597657f2b928eb/ros2.repos BUILD args: --packages-above-and-dependencies rclcpp TEST args: --packages-above rclcpp ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14752

fujitatomoya commented 4 weeks ago

@Mergifyio backport jazzy

mergify[bot] commented 4 weeks ago

backport jazzy

✅ Backports have been created

* [#2660 Fix NodeOptions assignment operator (backport #2656)](https://github.com/ros2/rclcpp/pull/2660) has been created for branch `jazzy`
fujitatomoya commented 4 weeks ago

i will backport this only to jazzy. (Iron is just one day away from E.O.L and humble does not have this code.)