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

Reply via email to