search

ros-drivers / ros2_ouster_drivers

ROS2 Drivers for the Ouster OS-0, OS-1, and OS-2 Lidars
https://ouster.com/
Apache License 2.0
139 stars 79 forks source link

Order headers with Google style #33

Closed rotu closed 4 years ago

rotu commented 4 years ago

Order headers with Google style https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

SteveMacenski commented 4 years ago

Hi,

I know you're trying to help here - but this is not something I'm going to merge. Call it my fear beat into me by the wrath of PCL about messing with include orders with ROS anywhere in the mix (plus just my workflow built upon it). But C headers, then libraries in order of specificity, sorted by libraries of origin is how I order these things so that I can scan through quickly.

There's a bunch of Google styling that ROS style guide doesn't follow - even internally at google this styling is loosely followed. There are plenty of examples in ROS2 that don't follow this and I'm not inclined to either due to aforementioned nightmare-ish experiences & styling.

rotu commented 4 years ago

Fair nuff. I'm not taking it personally. I do recommend at least ordering alphabetically, though.

SteveMacenski commented 4 years ago

Thanks, yeah I'm not sure if you'd had the nightmare-scene of issues around PCL, ROS, PCL wrappers, and include / STL headers, but it can take days (or weeks) to untangle for complex systems. I tend to steer clear of mixing things around due to the tree nature of includes (and no idea what the next guy after me / working above me will do). I'm especially concerned in repos like this that make use of not only PCL, but creating new templates for PCL.

image

rotu commented 4 years ago

Never have but I believe your experience.

rotu commented 4 years ago

Also, I think that by putting so much in headers and templates, and so little in cpp files, exacerbates the problem of include order hell. With delayed, compilation, missing or subtly interdependent header files are not reported until the first template instantiation (or worse, the second instantiation) that uses them!