search

orocos-toolchain / orogen

Code generator for components and type handling in Rock - the Robot Construction Kit - and the Orocos Toolchain
http://rock-robotics.org
Other
4 stars 35 forks source link

fix indentation and line lenghts in Task.hpp's template #134

Open doudou opened 4 years ago

doudou commented 4 years ago

Hi @g-arjones. Thanks for putting the finger where it hurts.

The whole vera thing has not delivered for me. It's very hard to extend and is unsupported by essentially anyone. For instance, it will complain about missing spaces after [ in a string because it believes it is an array.

I've started toying with using clang-format again. Someone had an interesting idea, that is to run clang-format on a copy of the file and let people look at the diff. I think there's something there, and that would definitely be a better way forward. I've come with a minimal google-based configuration that is close to existing code: https://gist.github.com/doudou/a240627b5921c1ca6321391629fae273

Why am I talking about it now ? I just realized I haven't fixed the .cpp files as well, which makes no sense (if we're fixing the templates, let's do it completely). I'd like to us to agree to a way with formatting (and a configuration) and then move forward with this change, this time making sure all the generated C++ code passes whatever formatting solution we picked.

If you agree, let's put this as draft, and move the discussion to the team discussion ?

g-arjones commented 4 years ago

Yeah, well, I have spent A LOT of time looking into all the options multiple times now and I really believe vera++ is the best one (cpplint is a close second option but it's not extendable at all) if we want "live" linting (and I agree it sucks, btw). I don't think turning clang-format into a linter is feasible if you want meaningful error/warning messages. The reality for me is that linting C/C++ is very hard and no one cares enough to try to fix the situation.

Now, what I believe would provide the best value (with a tradeoff in usability) is using clang-tidy and getting rid of vera++. It's highly customizable and has plenty of checks but requires the tool to be able to "build" the code. Since it supports compilation databases, the vscode extension could wait until it finds one to actually try to lint the code (that's the drawback in usability I mentioned before). It's supported by cmake with a flag so we could consider integrating with the CI through rock macros as well.

If you agree, let's put this as draft, and move the discussion to the team discussion ?

Fine :+1:

g-arjones commented 4 years ago

it supports compilation databases

Headers usually don't show up in those so there is that.. :(