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:29:00 GMT
Basically, hard enforcement (failing the build) makes us share
responsibility for quality control.


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

On Wed, Jan 7, 2015 at 4:28 PM, Christopher <ctubbsii@apache.org> wrote:

> 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