+1, +1, -1. #3 could be a +1 in the future. On Mon, Apr 11, 2016, 12:01 Leif Hedstrom <zw...@apache.org> wrote:
> Hi, > > James and I’ve been looking at some changes to clang-format, wanted to > take it here first. Just running the new version of clang-format produces a > patch of about 6,500 lines. This is mostly removal of superfluous empty > lines, and removing a space between a type cast and the variable. These are > all good changes IMO (basically bug-fixes in clang-format). > > In addition to that, we’re contemplating the following changes: > > 1) Change the maximum number of empty lines from “2” to “1”. This adds > about 10k to the patch size, and likely has little impact on being able to > cherry-pick across LTS versions. > > 2) Change the struct brace indentation to be the same as we do for > “class", i.e. > > struct Config > { > > instead of as it is today: > > struct Config { > > > This was not possible to do with the old version of clang-format, but it > is now. This adds about 10k lines to the patch. > > 3) clang-format has an option to “sort” #include directives in the source > files. This makes our builds fail in magnificent ways, but obviously we > could fix that. I don’t know how much work it would be, but likely much > more than we can expect to get done before 6.2? Unclear how large this diff > would be, since we have to manually fix a bunch of it. > > > My personal “votes” are > > 1: +1 > 2: +0 > 3: -0 > > > I’m only concerned about 3) from a time-to-market perspective, maybe that > can be a later project as part of some of the build cleanup that Jason K. > is working on? > > > Cheers, > > — Leif > >