impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Hecht <dhe...@cloudera.com>
Subject Re: Code formatting with clang-format
Date Thu, 18 Aug 2016 16:46:29 GMT
Is it easy to run over the whole backend? If so, why not do that and post a
gerrit review of the diff so people can spot check whatever files they are
interested in.

On Thu, Aug 18, 2016 at 9:37 AM, Tim Armstrong <tarmstrong@cloudera.com>
wrote:

> +1 to Jim's general approach.
>
> Maybe we should get a couple more people to try it out and give feedback? I
> tried it out on a source file I was familiar way.
>
> On Tue, Aug 16, 2016 at 10:29 AM, Jim Apple <jbapple@cloudera.com> wrote:
>
> > Oh, and I should note that this file, right now, only handles C++.
> > clang-format also works with Java, but that's future research.
> >
> > On Tue, Aug 16, 2016 at 10:09 AM, Jim Apple <jbapple@cloudera.com>
> wrote:
> > > I support incremental reformat, not bulk reformat.
> > >
> > > I think we should make this our canonical style, but I also think we
> > > should be willing to update it sometimes. I think when we do update
> > > it, we don't need to do a bulk reformat.
> > >
> > > On Tue, Aug 16, 2016 at 9:06 AM, Tim Armstrong <
> tarmstrong@cloudera.com>
> > wrote:
> > >> +1 for automating this. I think the style is pretty good even if it
> > doesn't
> > >> exactly match. I think it will save a lot of time wrapping lines, etc.
> > >>
> > >> What is the proposed approach to putting this into use? Will we just
> > >> incrementally reformat things as they're touched with
> git-clang-format,
> > or
> > >> try to do a bulk reformat?
> > >>
> > >> Will this be our canonical style? I.e. if a patch author or reviewer
> > >> doesn't like what clang-format does, do we just stick with the tool's
> > >> output for consistency.
> > >>
> > >> My preference is that we just do it incrementally and that we do make
> it
> > >> our canonical style.
> > >>
> > >>
> > >> On Tue, Aug 16, 2016 at 12:00 AM, Alex Behm <alex.behm@cloudera.com>
> > wrote:
> > >>
> > >>> +1 for abandoning some of our style idiosyncrasies in favor of
> > >>> easy-to-maintain automation
> > >>>
> > >>> On Mon, Aug 15, 2016 at 4:57 PM, Henry Robinson <henry@cloudera.com>
> > >>> wrote:
> > >>>
> > >>> > I think this is great - really useful to have. There are some
small
> > >>> > deviations from our traditional style (see the review for a couple
> of
> > >>> > them). They really don't bother me, and I think it's much better
to
> > have
> > >>> > automated formatting than to hang on to the position of a : in
a
> > for()
> > >>> > statement :) But I asked Jim if he'd start a thread here to check
> if
> > >>> others
> > >>> > agree.
> > >>> >
> > >>> > On 15 August 2016 at 15:18, Jim Apple <jbapple@cloudera.com>
> wrote:
> > >>> >
> > >>> > > I would like to have a clang-format config file in our directory
> to
> > >>> help
> > >>> > > new contributors understand how to format code and have a
tool to
> > do it
> > >>> > for
> > >>> > > them. Through the time I've been sending patches I've been
> > >>> accumulating a
> > >>> > > .clang-format file that seems to minimize the style comments
I
> > get. You
> > >>> > can
> > >>> > > see it here:
> > >>> > >
> > >>> > > https://gerrit.cloudera.org/#/c/3886
> > >>> > >
> > >>> > > And you can save it and upload it to play with here:
> > >>> > >
> > >>> > > http://zed0.co.uk/clang-format-configurator/
> > >>> > >
> > >>> > > I would love to hear your thoughts.
> > >>> > >
> > >>> >
> > >>> >
> > >>> >
> > >>> > --
> > >>> > Henry Robinson
> > >>> > Software Engineer
> > >>> > Cloudera
> > >>> > 415-994-6679
> > >>> >
> > >>>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message