Use clang format for style checking #155
Conversation
|
I think we should use this as a general guideline btw. We should not reject every PR that fails the formatting check, because in some cases a strict application of the rules just doesn't produce well formatted code (i.e. sometimes manual formatting is better). So we'll just have to use sensible judgment for this. @paoloambrosio what's your opinion on this? |
|
Unless I've done something stupid, currently the format is wrong for 17 files. Are we happy with the changes that We should also add instructions on how to check code formatting before sending a PR (perhaps in the contributing guidelines?). Apart from this, I think it's a great addition 👍 |
Really? I get 76 files: $ clang-format-4.0 --style=file -i (^([Bb]uild*|include))/**/*.(cpp|hpp|h) include/cucumber-cpp/**/*.(cpp|hpp|h)
$ git diff --stat
...
76 files changed, 1108 insertions(+), 1190 deletions(-)
Most of these changes are the result of our currently inconsistent formatting in the code base. I've tried multiple variations of options and tried to find the selection that leads to the smallest amount of differences. Most of the formatting that
I'm not sure that's a good idea, that would reduce the usefulness of
Agreed. |
|
@muggenhor what about this? Are you going to rebase and merge? In general I agree that we shoul apply this only to new commits, not to rewrite history. |
This config tries to stick as close as possible to the current style.
It is possible to deviate from this (strict) set of rules by surrounding
a block of code by:
// clang-format off
void unformatted_code ( );
// clang-format on
Reformatting all lines (or just those selected with the `-lines`
parameter) can be done with:
clang-format -i $file
Reformatting all lines touched by the last commit with:
git clang-format --style=file --commit=HEAD~1
This addresses #151
Add a Travis CI check that will fail if _changed_ code doesn't adhere to this configured style. It will also display the changes required to adhere to the style. This fixes #151
|
Changes Unknown when pulling 28132a1 on muggenhor:clang-format into ** on cucumber:master**. |
This first adds a clang-format configuration that should match our current style as much as possible. Subsequently another Travis build gets added that uses this config to verify that all changes (when compared to the last merge from master) in a branch (including pull requests) adhere to that style. Any differences will cause that build to fail and the differences with the style, as clang-format would format it, to be shown as a highlighted unified diff.
The last commit of this PR is present only to demonstrate how such a failure to adhere to the style gets reported, that commit itself should not be merged back into master.muggenhor@8b20bbeac3331c3d8ad4443d981d1e65806742fa demonstrates how such a failure looks like. That failure looks like this: https://travis-ci.org/muggenhor/cucumber-cpp/jobs/266237870This resolves #151.