granular rclcpp/rclcpp.hpp and include what you use - #454
Conversation
Signed-off-by: Alejandro Hernandez Cordero <ahcorde@gmail.com>
fujitatomoya
left a comment
There was a problem hiding this comment.
lgtm with green CI.
|
Pulls: #454, ros2/demos#806, ros2/message_filters#328, ros2/rosbag2#2483, ros2/realtime_support#151, ros/robot_state_publisher#254, ros2/geometry2#984, ros2/ros2_tracing#263, ros2/rmw_zenoh#1043, ros2/rclcpp#3249, ros2/rviz#1854 |
wjwwood
left a comment
There was a problem hiding this comment.
I'm on the fence about the trade-off of avoiding rclcpp.hpp here in the examples. I can see the argument for doing it even here, but I also think the impact to user experience is unfortunately. Specifically I'm concerned about them feeling like they need to know (or know how to figure out) which headers they need in which situations, and while copying an example is fine, when they start to expand the scope of the example they copied (these are meant to be starting points for users to work off of) I'm afraid they'll struggle to know what they need to include and in which situations.
Tooling (IWYU linters) and AI agents might make this less of a concern these days though. On the other hand, I'd like to keep the "lightweight, ai-free" user archetype in mind as long as we can.
Overall, I'm ok with this change, but I think we should be thoughtful about it. Maybe there's a middle ground where we have some headers for common use cases that keep compile time impacts in mind. Either way, that idea (and others) are out of scope for this pr.
Description
Granular rclcpp/rclcpp.hpp also in documentation
Did you use Generative AI?
Claude Opus 4.8