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 <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