Problem: inconsistent formatting #2908
Conversation
Solution: added .clang-format file
Solution: applied clang-format
Solution: add clang-format to cmake, and add another travis-ci build type
Solution: restrict to src, tests, perf, tools and include directories
Solution: explicitly select clang-5.0
Solution: fix execution directory
Solution: fix CMAKE_MODULES_PATH
sigiesec
force-pushed
the
clang-format
branch
from
c19df88
to
43c11d0
Compare
|
@sigiesec is this ready to merge or still subject to discussion? |
|
It is ready to merge from my point of view. I discussed with @bluca and @jimklimov at the Hackathon. @bluca just wanted to wait if @somdoron has any objections/suggestions. |
|
However, please do not merge any other PR before this, as this would most probably require modifying this PR, and indirectly all my others that are based on the changes in this. |
|
@somdoron what do you think? Having the CI addition should help contributors. Also we can extend the PR template with a note. If we go with this, I think it's better this way with the huge diff, biting the bullet in one go, rather than bit-by-bit. We can tag the repository with something like "pre_reformat" to make it easier to use git blame. |
|
Oh **** I didn't notice the compiler snippets were touched by this as well. Sorry! Let's see if it can be fixed in a quick way by the gui |
|
@sigiesec really sorry - I shouldn't do things when I'm really tired. Please feel free to use the same PR for the other commits as well, in a single branch, so that your life is easier - it doesn't make much difference anyway if they are in a single PR |
|
@sigiesec thought of a way to fix it - I temporarily reverted the other PR, so that there are no merge conflicts here anymore. Phew! |
|
@somdoron ping :-) |
|
@bluca: somdoron appears not to be available :( so would you mind merging the PR now? It is difficult for me and any others working on some changes to go on with this pending. |
|
I've tried to annoy him via email now - let's wait a couple more days first, then I'll merge, ok? |
|
Sorry, missed it. Looks fine to me, hard to review so many files. How we will prevent new PR from using wrong formatting? |
|
Correct formatting is checked by a new travis job. Here you can see an example of output in case of a formatting failure: https://travis-ci.org/ZMQers/libzmq/jobs/336505439#L624 |
|
@bluca So can you merge it now? Then it is also easier to give the other pending PRs a closer look. |
|
Done! I think the other branches will need rebasing now? Github adds a merge commit |
|
Great, thanks! Hm... Maybe ... I hoped that this would not be necessary as long as there are no conflicts, but it seems it does not rebase automatically. I will have a look at the other PRs tomorrow. |
Solution: defined formatting using .clang-format file, set up CI checks for correct formatting, and initially applied reformatting to all source files