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 <jbap...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbap...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: No
