fix most cpplint and uncrustify errors

[mikaelarguedas]

This enforces google styleguide.
What is not enforced in this PR:

• removal of using namespace
• line length enforcement
• include order
• mark single-argument constructors as explicit

[mikaelarguedas]

@ros-pull-request-builder retest this please

[mikaelarguedas]

prereleased passed on indigo and kinetic, merging this

[jasonimercer]

@mikaelarguedas , this PR breaks ROS style pretty severely. Why was it decided to apply a different style? I can provide a clang-format file we use at Clearpath that implements the ROS style pretty closely if you like.

[mikaelarguedas]

@jasonimercer Thanks for the feedback.

This PR applied the ROS 2 style here to keep the diff between the 2 branches manageable.

I didnt realize it was breaking ROS 1 style significantly, I (wrongly) assumed that the ROS2 style was just more restrictive than its' ROS1 counterpart but not conflicting.

I can provide a clang-format file we use at Clearpath that implements the ROS style pretty closely if you like

Yes I would be interested in seeing the differences or at least pin down the rules that are not compatible between the 2 style guides.
From what I see the main difference is the placement of curly brackets for code blocks. Do you have any other ROS1 rule that is obviously broken by this PR?

[jasonimercer]

:( I didn't know the ROS 2 style was so different. Thanks for pointing me towards it.

This is the clang format we've been using in Navigation & Controls at Clearpath:

clang-format.txt

We based it on

https://github.com/davetcoleman/roscpp_code_format

before they pulled in MoveIt's version.

[mikepurvis]

The brace style between ROS 1 and ROS 2 is completely different— in ROS 1 style, the mandate was never to cuddle:

http://wiki.ros.org/CppStyleGuide#Formatting

The long-standing policy in core repos like ros_comm has been to avoid code style fixes in the name of not breaking patching and porting efforts, which is exactly what this change is doing for us (we have a fork of actionlib for a variety of reasons).

Has the policy changed? @dirk-thomas @tfoote

[dirk-thomas]

I think it is mostly a decision of each maintainer rather than a "general policy".

Especially in ros_comm with separate branches for each ROS distro that is certainly something I follow. For that repository I do see a window of opportunity coming up in about a year: when Indigo and Lunar are about to be EOLed and before N-turtle is started there are only two ROS distros out there at the same time (Kinetic and Melodic). During that phase it might be doable to perform style cleanup (on both branches). Of course that would still imply extra effort on forks...

[mikaelarguedas]

@mikepurvis @jasonimercer Sorry for the late response.

As this issue will span much more than this repository in the future (all ros_base and robot_model packages having been converted to the ROS 2 style already), I wanted to flush out all the conflicts between the style guides and come up with a way forward to reconcile the styles rather than just reverting this. There is a proposal of ROS 1 style relaxation available at https://discourse.ros.org/t/rfc-proposal-to-reconcile-ros-1-and-ros-2-c-style-guides/4616, any feedback is welcome.

The long-standing policy in core repos like ros_comm has been to avoid code style fixes in the name of not breaking patching and porting efforts, which is exactly what this change is doing for us (we have a fork of actionlib for a variety of reasons).

My understanding was that this policy was for maintainers maintaining several branches of their ROS 1 repositories (rather than accommodating people maintaining a fork). This was actually the motivation for this change as I wanted the style cleanup to be performed before branching out for melodic.

I understand the pain of having to convert your patches. If you have anything in this patches that deserve being contributed upstream, please consider doing so, as this can reduce the friction when things change on the upstream repository.
When looking at the forks before merging this, I didn't see significant blockers for rolling this out. I came to the same conclusion as @mikepurvis when the Python style was updated (#43 (comment)).
I acknowledge that the Python change was complying with the ROS 1 style while this one did not.

I assume that by now the matching changes have been performed on your side, let me know if this is not the case and I can look into reverting the brace changes to reduce conflict resolution burden.