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 22:12:06 GMT
Some of the checkstyle rules catch stuff in javadocs, which even I don't
typically think to look for (like invalid HTML), and I'm pretty pedantic
already when it comes to javadocs.


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

On Wed, Jan 7, 2015 at 4:36 PM, Josh Elser <josh.elser@gmail.com> wrote:

> +1 for that. I feel bad when I see you constantly come across after my
> commits when I inevitably bork some Javadoc.
>
>
> Christopher wrote:
>
>> 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