accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher <ctubb...@apache.org>
Subject Re: [VOTE][LAZY] Format all supported branches
Date Wed, 07 Jan 2015 21:28:07 GMT
Fail (which is what I think we want, especially for the broken javadoc
issues I've been seeing), but it could be configured either way. What I
wouldn't want to happen is for them to be printed and simply ignored. If
we're going to have to go back and fix them (instead of ignoring), I'd
rather they fail, so the person who introduced the problem is held
responsible for fixing before they push. The plugin can also be skipped, or
configured to not fail, with command-line options.

One great thing about Checkstyle rules, is they have *very* good
descriptions about what is wrong. So, when you do see an error, it is
typically a very obvious fix... and they give you the line number, too.


--
Christopher L Tubbs II
http://gravatar.com/ctubbsii

On Wed, Jan 7, 2015 at 4:12 PM, Mike Drob <madrob@cloudera.com> wrote:

> Will the checkstyle rules fail the build or just print warnings?
>
> On Wed, Jan 7, 2015 at 3:11 PM, Christopher <ctubbsii@apache.org> wrote:
>
> > In the long run, the rules will (hopefully) save me work following up
> > fixing bad javadocs and trivial warnings.
> >
> >
> > --
> > Christopher L Tubbs II
> > http://gravatar.com/ctubbsii
> >
> > On Wed, Jan 7, 2015 at 4:03 PM, Eric Newton <eric.newton@gmail.com>
> wrote:
> >
> > > +0
> > >
> > > I don't think it's worth the disruption, but I don't mind if you're
> going
> > > to do all the work.
> > >
> > >
> > > On Wed, Jan 7, 2015 at 3:47 PM, Mike Drob <madrob@cloudera.com> wrote:
> > >
> > > > Also, if you're using Eclipse to do the auto-format, please check for
> > > > trailing white-space on otherwise empty javadoc lines.
> > > >
> > > > If you automate this in some fashion outside of Eclipse (because
> other
> > > > people may prefer other editors), this would be a useful script to
> add
> > > to a
> > > > top-level dev-tools folder.
> > > >
> > > > On Wed, Jan 7, 2015 at 2:36 PM, David Medinets <
> > david.medinets@gmail.com
> > > >
> > > > wrote:
> > > >
> > > > > +1
> > > > >
> > > > > Are you automating the process so that you can re-apply the same
> > steps
> > > > > in one year?
> > > > >
> > > > > On Wed, Jan 7, 2015 at 3:31 PM, Christopher <ctubbsii@apache.org>
> > > wrote:
> > > > > > Can do. It's a bit more work for me, because I have to repeat
the
> > > same
> > > > > > actions over and over again, but if it helps history look a
> little
> > > > > cleaner,
> > > > > > i can do it, and just stick to -sours and repeat for the next
> > > branch..
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Christopher L Tubbs II
> > > > > > http://gravatar.com/ctubbsii
> > > > > >
> > > > > > On Wed, Jan 7, 2015 at 3:25 PM, Mike Drob <madrob@cloudera.com>
> > > wrote:
> > > > > >
> > > > > >> Please do not do formatting during merge conflict resolution,
> and
> > > make
> > > > > >> those be separate commits.
> > > > > >>
> > > > > >> On Wed, Jan 7, 2015 at 2:23 PM, Josh Elser <
> josh.elser@gmail.com>
> > > > > wrote:
> > > > > >>
> > > > > >> > ack'ed
> > > > > >> >
> > > > > >> >
> > > > > >> > John Vines wrote:
> > > > > >> >
> > > > > >> >> +1
> > > > > >> >>
> > > > > >> >> On Wed, Jan 7, 2015 at 3:12 PM, Christopher<
> > ctubbsii@apache.org>
> > > > > >> wrote:
> > > > > >> >>
> > > > > >> >>  To make it easier to apply some minimal checkstyle
rules for
> > > > > >> >>> ACCUMULO-3451,
> > > > > >> >>> I'm announcing my intentions to do a full,
one-time,
> > auto-format
> > > > and
> > > > > >> >>> organize imports on all our supported branches
(1.5, 1.6,
> and
> > > > > master)
> > > > > >> to
> > > > > >> >>> bring us up to some degree of compliance with
our
> agreed-upon
> > > > > >> formatting
> > > > > >> >>> standards.
> > > > > >> >>>
> > > > > >> >>> Benefits:
> > > > > >> >>> To have additional checks, in particular against
javadoc
> > > problems
> > > > > and
> > > > > >> >>> other
> > > > > >> >>> common trivial warnings in the build.
> > > > > >> >>> To ensure less divergence from our agreed-upon
formatting
> > > > standards.
> > > > > >> >>> Formatting first makes it much less tedious
and easier on me
> > to
> > > > add
> > > > > >> these
> > > > > >> >>> checks to the build.
> > > > > >> >>>
> > > > > >> >>> Issues I've considered:
> > > > > >> >>> I will deal with all the merge conflicts.
> > > > > >> >>> I will ignore generated thrift code.
> > > > > >> >>> Conflicts with new code in people's branches
should be
> minimal
> > > > (and
> > > > > >> >>> easily
> > > > > >> >>> resolved by formatting according to our standards).
> > > > > >> >>> Regarding concerns about history tracking,
in general, each
> > > format
> > > > > >> change
> > > > > >> >>> is small, but they are numerous. So, the impact
on tracking
> > > > history
> > > > > >> >>> should
> > > > > >> >>> be very minimal (you'll see things like a brace
moved to the
> > > same
> > > > > line
> > > > > >> as
> > > > > >> >>> the else statement it is associated with...
stuff that won't
> > > > > generally
> > > > > >> >>> affect your ability to debug).
> > > > > >> >>> I'll also do a "format only" commit, separately
from any
> > > > substantive
> > > > > >> >>> changes regarding the rule changes, so the
mass formatting
> > > change
> > > > > will
> > > > > >> >>> happen in one place, and it will also be easy
to revert, if
> > > > > absolutely
> > > > > >> >>> necessary.
> > > > > >> >>>
> > > > > >> >>> I'll give this 24 hours (it can be reverted
if somebody
> > objects
> > > > > after
> > > > > >> >>> that).
> > > > > >> >>>
> > > > > >> >>> --
> > > > > >> >>> Christopher L Tubbs II
> > > > > >> >>> http://gravatar.com/ctubbsii
> > > > > >> >>>
> > > > > >> >>>
> > > > > >> >>
> > > > > >>
> > > > >
> > > >
> > >
> >
>

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