impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jim Apple (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Add .clang-format for Impala's C++ style
Date Mon, 15 Aug 2016 22:15:55 GMT
Jim Apple 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.

 > 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 <jbapple@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: No

Mime
View raw message