Lars Volker has posted comments on this change.
Change subject: Add .clang-format for Impala's C++ style
......................................................................
Patch Set 1:
> > I ran this over a few patches. Some small things I noticed:
> >
> > 1. The arg-breaking behaviour is different: the formatter prefers
> > to put all arguments on one line even if that means adding a
> > newline before the first one.
>
> BinPackParameters can change this, along with
> AllowAllParametersOfDeclarationOnNextLine.
> If both are false, instead of all parameters being on the next
> line, every parameter will get its own line. That doesn't seem
> closer to our style, though. There may not be a closer clang-format
> config for this.
I noticed that AlignAfterOpenBracket = DontAlign only does this if the
parameters fit into their own line. Once they exceed a single line's length
they get wrapped properly:
std::copy(listaaaaaaa, liaaaaaaast + 9,
std::ostream_iterator<int>(std::cout, " "));
Maybe a bug in clang-format.
>
> > 2. for (auto& foo: bar) -> for (auto& foo : bar). I don't care
> > about this one, all the same to me.
>
> I don't see a way to configure this one, unfortunately.
>
> > 3. Indentation on constructor member initialisation lists is by
> two
> > spaces, not four (i.e. line up with the ':'). Again, I don't care
> > about that.
>
> This seems to match, for instance, analytic-eval-node.cc -- the
> colon is two space from the left margin (i.e. starts in column 3),
> and the member variables start in column 5.
>
> This is what I get with this config with
> http://zed0.co.uk/clang-format-configurator/
>
> I think I must be misunderstanding.
>
> > I don't see any problems with these small changes, but you might
> > want to socialise this a bit on impala-dev@ before committing the
> > formatter. It would be excellent for this to be as close to our
> > canonical style as possible, even if that means changing our
> > canonical style a bit.
>
> OK, I'll email dev@.
--
To view, visit http://gerrit.cloudera.org:8080/3886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I274c5993c7be344fc4b7729d21a13da993f9f3aa
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: No
