search

magazino / move_base_flex

Move Base Flex: a backwards-compatible replacement for move_base
https://uos.github.io/mbf_docs/
BSD 3-Clause "New" or "Revised" License
429 stars 154 forks source link

Add clang format and do not git-ignore it #239

Closed corot closed 3 years ago

corot commented 3 years ago

Based on https://github.com/davetcoleman/roscpp_code_format/blob/master/.clang-format

Rayman commented 3 years ago

Are you planning to clang-format the full repository, or is it already formatted?

corot commented 3 years ago

Are you planning to clang-format the full repository, or is it already formatted?

mmm.... :thinking: not really; code is mostly formatted and I don't want to destroy manually aligned code. But I can highlight format errors on CLion.

dorezyuk commented 3 years ago

if you want to exclude stuff from being formatted by clang then wrap it in

// clang-format off
.. bla
// clang-format on
corot commented 3 years ago

Shit... I should have done this after merging open PRs... :frowning:

corot commented 3 years ago

This destroys the git history for individual lines. This is not good. We should have a format checker only for PRs. Please do not reformat the whole repo. I am absolutely against that.

True, true... but ignoring white-spaces the history is still recognizable, don't you think? Is there any way to force git to ALWAYS ignore white-spaces?

Rayman commented 3 years ago

You can configure the CI to only verify the format of the current pull request. That way the code get's incrementally better formatted. See clang-format-diff

corot commented 3 years ago

I reverted the formatting; now just add the clang format file, so we can configure CI to check if new PRs adhere to it

spuetz commented 3 years ago

Could you also change the CI script to use https://clang.llvm.org/docs/ClangFormat.html, e.g. like this here: https://gitlab.gnome.org/GNOME/glib/-/blob/0ea992a9d8378189868c27c181ee00e390f5f346/.gitlab-ci/run-style-check-diff.sh and here: https://gitlab.gnome.org/GNOME/glib/blob/0ea992a9d8378189868c27c181ee00e390f5f346/.gitlab-ci.yml#L33-39

corot commented 3 years ago

Could you also change the CI script to use https://clang.llvm.org/docs/ClangFormat.html, e.g. like this here: https://gitlab.gnome.org/GNOME/glib/-/blob/0ea992a9d8378189868c27c181ee00e390f5f346/.gitlab-ci/run-style-check-diff.sh and here: https://gitlab.gnome.org/GNOME/glib/blob/0ea992a9d8378189868c27c181ee00e390f5f346/.gitlab-ci.yml#L33-39

mmm... I added both scripts to the PR and run the first on .travis.yml, but I have no idea how to test if this will work as we expect. Any help will be welcome