Conversation
|
I like the additional benefit of removing dependence on python here for such a simple action. I'm going to add this to the PMC agenda for this week, thanks. |
fujitatomoya
left a comment
There was a problem hiding this comment.
LGTM, IMO this is good enhancement for maintainability. besides less dependency, this also helps me to walk through the source code symbols without building the em files 👍
sloretz
left a comment
There was a problem hiding this comment.
This seems like a great spot to not generate code. As a user I've gone looking for these macros in the past, and it took me extra time to figure out what macros are being generated.
|
What about the |
That's a reasonable followup, yeah. That one should be a bit easier; it doesn't too much of its own machinery (mostly wrapping of these macros). |
|
Pulls: #502
|
|
One other thing I should mention; I don't think this should be backported to stable distributions. While it doesn't change the user-facing macros, there are technically some macros (like |
|
Ah. So it looks like we actually have to do rclcpp right now, because it depends on this (I didn't see it locally because I didn't rebuild from scratch, and I must have had the python code still around in my install space). I'll go ahead and do rclcpp first, then we can revisit this. |
In my opinion, this makes it far more readable for humans, and thus easier to update/maintain in the future. This ends up removing ~500 lines of python code, empy templates, and associated machinery, plus 1500 lines of generated code. This also has the the nice side effect of making it so that rosdoc2 will generate the documentation for all of these macros. Signed-off-by: Chris Lalancette <clalancette@gmail.com>
faecb49 to
c1ea754
Compare
All right, I've updated this PR and opened up ros2/rclcpp#2870 , which does the same for rclcpp. |
|
This pull request has been mentioned on ROS Discourse. There might be relevant details there: https://discourse.ros.org/t/ros-pmc-minutes-for-june-10-2025/44219/1 |
|
Pulls: #502, ros2/rclcpp#2870
|
|
This pull request has been mentioned on Open Robotics Discourse. There might be relevant details there: |
|
CI is clean here, and there were no concerns in the discourse thread, so I'm going ahead and merging in this one and ros2/rclcpp#2870 |
Description
In my opinion, this makes it far more readable for humans, and thus easier to update/maintain in the future. This ends up removing ~500 lines of python code, empy templates, and associated machinery, plus 1500 lines of generated code. This also has the the nice side effect of making it so that rosdoc2 will generate the documentation for all of these macros.
Is this user-facing behavior change?
No (API remains the same, only the implementation is different)
Did you use Generative AI?
No.
Additional Information
I've compiled all of the ROS 2 core with this in place, and it seems happy enough.