ament / ament_lint

Apache License 2.0
38 stars 107 forks source link

`ament_clang_format` and `colcon test` #425

Open Myzhar opened 2 years ago

Myzhar commented 2 years ago

I'm trying to format the C++ code of my node by using ament_clang_format.

The problem is that the code formatted with ament_clang_format --reformat conflicts with what's expected by colcon test.

There are many formatting rules used by ament_clang_format that are not the same:

scheunemann commented 2 years ago

There are a few known issues with, e.g., if-else that are contradicting: https://github.com/ament/ament_lint/issues/158 other than that reformatting via clang-format and running the standard tests of ament_lint_common works for me.

Btw, if set up like a standard package, colcon test looks into your package.xml to retrieve the tests you wish to run. You could also specify the following to run clang-format:

  <test_depend>ament_lint_auto</test_depend>
  <test_depend>ament_cmake_clang_format</test_depend>
  <test_depend>ament_cmake_clang_tidy</test_depend>

The defacto default is:

  ...
  <test_depend>ament_lint_auto</test_depend>
  <test_depend>ament_lint_common</test_depend>
  ...
Myzhar commented 2 years ago

The defacto default is:

  ...
  <test_depend>ament_lint_auto</test_depend>
  <test_depend>ament_lint_common</test_depend>
  ...

That's indeed what I usually do.

What I noticed is that ament_uncrustify --reformat provides a code more similar to what colcon test expects, even if there are still a few differences.

Does it make sense to have two possibilities with ament_uncrustify and ament_clang_format?

scheunemann commented 2 years ago

I usually do ament_clang_format --reformat ** ament_uncrustify --reformat and change the leftover bits per hand. it is known that both do give different results (see https://github.com/ament/ament_lint/issues/158), I think it is just not a priority currently to align them since there is a workaround for now.

roncapat commented 1 year ago

Is the intended way forward to align clang-format to uncrustify or vice-versa?