approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
318 stars 51 forks source link

Add .clang-format file to format code consistently #39

Closed claremacrae closed 4 years ago

claremacrae commented 5 years ago

We have a style guide of sorts:

https://github.com/approvals/ApprovalTests.cpp/blob/master/tests/Catch2_Tests/StyleGuide.h

But nothing to help us keep the formatting consistent...

In another project, I set up a .clang-format file to try to match the Style Guide.. https://github.com/claremacrae/ApprovalTests.cpp.Nursery/blob/master/.clang-format

In particular, as we increase the use of namespaces, e.g. in #34, I would like them to be indented so that namespace nesting can be spotted at a glance...

I know that @alastairUK has similarly been experimenting with this, and preferred not limiting the line length.

This is one of those things where any style definition is going to be better than having something inconsistent...

I would hold off implementing this until all the current PRs and open branches on forks have been resolved, though.

claremacrae commented 5 years ago

Here is @alastairUK's suggested file, renamed to allow me to upload it here:

clang-format-alastairUK.txt

alastairUK commented 5 years ago

I had a go at applying a format in this branch https://github.com/alastairUK/ApprovalTests.cpp/tree/clang-format

I think it looks OK. I used your clang-format with the changes in this commit:

https://github.com/alastairUK/ApprovalTests.cpp/commit/db76013964a6ab440561933f957ac8a12fed55e6

There were a couple of issues after sorting the includes but I believe the fixes are obvious.

Have also added a github action to apply clang-format after a push. It will fail if changes detected. It can likely be merged into the build action at some point...

claremacrae commented 5 years ago

Thanks for this - really helpful.

JayBazuzi commented 5 years ago

On one project I added clang-format validation to CI, so I can be sure that every PR is compliant. Here's how: https://github.com/JayBazuzi/Okra/blob/master/.travis.yml

claremacrae commented 5 years ago

Thanks Jay, that's really helpful - I learned some sh/bash stuff too...

alastairUK commented 5 years ago

https://github.com/alastairUK/ApprovalTests.cpp/commit/7d5b45955617e6b1adda7d4d1b4ebd1757939758

This was my (poor) attempt to do this. I think it can likely be added to:

https://github.com/approvals/ApprovalTests.cpp/pull/35/commits/9adec81a12defcfa296a78529fc8261dd22b6744

To do this step after running the various build (or perhaps before?)

It should fail if there are any formatting issues (at least on my tests I did)

It would be handy to show the formatting changes needed in the (failing) pull request (for instance) but no idea how to do that!

claremacrae commented 4 years ago

This is useful for experimenting with clang format settings: https://zed0.co.uk/clang-format-configurator/

claremacrae commented 4 years ago

@dheater is kindly going to take a look at this!