Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ament_clang_format and colcon test #425

Open
Myzhar opened this issue Nov 18, 2022 · 4 comments
Open

ament_clang_format and colcon test #425

Myzhar opened this issue Nov 18, 2022 · 4 comments
Labels

Comments

@Myzhar
Copy link

Myzhar commented Nov 18, 2022

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:

  • includes order
  • brackets position
  • ...
@scheunemann
Copy link

There are a few known issues with, e.g., if-else that are contradicting: #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
Copy link
Author

Myzhar commented Nov 28, 2022

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
Copy link

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 #158), I think it is just not a priority currently to align them since there is a workaround for now.

@roncapat
Copy link

roncapat commented May 8, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants