Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: <CAC-pSX2XK00tkyNKUuLkF9i-=VZRA5uHjRFtYOKhZaPdV4LjRw@mail.gmail.com>
References: <CAM_aD9dfq3sF9eGFkADYXOECscVLw4mx-W0A9541c80P5cdy6w@mail.gmail.com>
 <CAC-pSX1VEGBA8G8yJ765VKqBg6Q7bPbU1GaqiAzrWngYTTJ1hg@mail.gmail.com>
 <CAEoRBeytnMxUbrYG3k+CDQsETscXrzyHf7piGfxBq7mBEjTtqA@mail.gmail.com>
 <CAEoRBex-NS3NmhEhtcq6HGEZe49e-cfkE7E8L6qJMZ8yAr8V3g@mail.gmail.com> <CAC-pSX2XK00tkyNKUuLkF9i-=VZRA5uHjRFtYOKhZaPdV4LjRw@mail.gmail.com>
From: Todd Lipcon <todd@cloudera.com>
Date: Sat, 3 Sep 2016 19:28:48 -0700
Message-ID: <CADY20s6L46x8dkH=z5mu43cEB5H8ijWQph0xk_WDwBRWY-zEKw@mail.gmail.com>
Subject: Re: Request for feedback: C++ Style Guide
To: "dev@impala" <dev@impala.incubator.apache.org>
Content-Type: multipart/alternative; boundary=001a1147da7ef51f76053ba55229
archived-at: Sun, 04 Sep 2016 02:28:57 -0000

--001a1147da7ef51f76053ba55229
Content-Type: text/plain; charset=UTF-8

Agreed that the lint script is a bit hacky, but it has very few false
positives, mostly false negatives. We run that lint script on every patch
in kudu. (Though tuned to disable a few of the more nit picky rules)

Todd

On Sep 2, 2016 10:24 AM, "Jim Apple" <jbapple@cloudera.com> wrote:

> I'm in favor of that. It's also worth mentioning that .clang-format is
> about what we do going forward, and not carte blanche to send patches
> that reformat whitespace in files written before we had .clang-format.
>
> Some of the style guide, of course, refers to non-whitespace based
> rules. Google has a linter script that tries to find divergence form
> the standard, but I am dubious of it:
>
> https://github.com/google/styleguide/blob/gh-pages/cpplint/cpplint.py
>
> It has known places where it is inaccurate, and it doesn't understand the
> AST.
>
> On Fri, Sep 2, 2016 at 10:03 AM, Tim Armstrong <tarmstrong@cloudera.com>
> wrote:
> > I'm not sure that the pros and cons are that enlightening. May make sense
> > to just remove them - not sure what others think.
> >
> > For formatting, I think we should consider leaning more on clang-format.
> > The discussion around clang-format seemed to be going that direction.
> >
> > E.g. maybe the rule should be something like "Our .clang-format is the
> > source of truth for how to deal with whitespace, except when
> clang-format's
> > output greatly diverges from the existing code style or common sense. In
> > that case, we should update the .clang-format file."
> >
> > On Fri, Sep 2, 2016 at 9:59 AM, Tim Armstrong <tarmstrong@cloudera.com>
> > wrote:
> >
> >> Yes, several things are completely wrong. E.g. we never use c-style
> casts.
> >>
> >> On Fri, Sep 2, 2016 at 9:38 AM, Jim Apple <jbapple@cloudera.com> wrote:
> >>
> >>> I left a comment on the page - I'm not sure how much these reflect our
> >>> actual current practice.
> >>>
> >>> On Fri, Sep 2, 2016 at 9:36 AM, Lars Volker <lv@cloudera.com> wrote:
> >>> > After some confusion in reviews about how to format code I moved our
> >>> > internal C++ Style Guide wiki page to the Apache wiki and updated all
> >>> links
> >>> > in it. You can find it here:
> >>> >
> >>> > https://cwiki.apache.org/confluence/pages/viewpage.action?
> >>> pageId=65868536
> >>> >
> >>> > At some point in time someone seems to have started a list of pro and
> >>> cons,
> >>> > some of which are worded rather negative. Do we want to revisit those
> >>> > comments or the style guide even?
> >>> >
> >>> > I'm looking forward to any feedback. Thanks, Lars
> >>>
> >>
> >>
>

--001a1147da7ef51f76053ba55229--