Apply clang-format to the code base #255
Conversation
Neverlord
force-pushed
the
topic/neverlord/clang-format
branch
from
d8b754a
to
87b96a4
Compare
|
I've ended up excluding the code under |
bindings/python/set_bind.h
Outdated
| std::is_copy_constructible<typename Set::value_type>::value, Class_> &cl) { | ||
| template <typename, typename, typename... Args> | ||
| void set_if_copy_constructible(const Args&...) { | ||
| } |
There was a problem hiding this comment.
Prefer to have empty brace sets on the same line.
| .def("publish_batch", | ||
| [](broker::publisher& p, std::vector<broker::data> xs) { p.publish(xs); }) | ||
| .def("publish", (void(broker::publisher::*)(broker::data d)) | ||
| & broker::publisher::publish) |
There was a problem hiding this comment.
I'm not sure why this is separating the & from the variable name, if the intention is for it to be taking the address of that variable.
There was a problem hiding this comment.
I'm not sure either was clang-format is thinking about this code. Maybe it confuses this with an argument declaration. Although, then it should put the & after the brace... 🤷🏻♂️
|
|
||
| // Wait until connection is established. | ||
| for ( bool has_peer = false; !has_peer; ) { |
There was a problem hiding this comment.
This was a weird mix of the Zeek style and whatever broker uses by default. There's actually a flag you can pass to clang-format to preserve this style, if that's what we want to do (SpacesInConditionalStatement). It doesn't look like we prefer this format everywhere else though, so I'm fine with letting it go.
Neverlord
force-pushed
the
topic/neverlord/clang-format
branch
from
1808f44
to
f331e55
Compare
|
@timwoj I've tweaked the coding style based on your suggestions:
I've also tweaked the penalties so that Let me know if it needs further tweaking. If you prefer, I can also squash the commits before a merge (kept the second commit for now to allow you to review the changes). |
|
Also, should've mentioned that his PR relates #249. |
Neverlord
force-pushed
the
topic/neverlord/clang-format
branch
from
f331e55
to
a4e6e6c
Compare
Neverlord
force-pushed
the
topic/neverlord/clang-format
branch
from
a4e6e6c
to
7e11326
Compare
For #255, we ran `clang-format` over the code base. By accident, we've also formatted `BrokerConfig.cmake.in` and broke CMake packaging in the process by changing `@PACKAGE_INIT@` to `@PACKAGE_INIT @`.
Aside from the obvious benefit of forcing a consistent coding style, this PR is also a preparation step for running
clang-tidy. Someclang-tidychecks can automatically provide a fix for certain findings. However, when applying such tool-assisted code changes, we also need to runclang-formatafterwards to make sure the code stays consistently formatted.