hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Purtell <andrew.purt...@gmail.com>
Subject Re: Better code quality with more checkstyles
Date Wed, 18 Nov 2015 01:25:36 GMT


> On Nov 17, 2015, at 5:13 PM, Stack <stack@duboce.net> wrote:
> 
> Want to make a patch Appy? Will up our violations count but the set looks
> reasonable to me.
> St.Ack
> 
>> On Tue, Nov 17, 2015 at 4:54 PM, Apekshit Sharma <appy@cloudera.com> wrote:
>> 
>> I agree. Sorry if I my message was lost in the boring sunday story, but
>> the proposal is only to enable new checkstyles which are listed in the
>> first mail so we can know current state of our codebase and gauge its
>> improvement overtime.
>> 
>> 
>>> On Tue, Nov 17, 2015 at 1:20 PM, Nick Dimiduk <ndimiduk@gmail.com> wrote:
>>> 
>>> The problem with a fixup "rampage" is it will very like introduce
>> conflicts
>>> with any patches that are in review process. We have no issue with
>> general
>>> code cleanup. The problem is introducing more work for
>>> contributors/reviewers while it's happening. The cleanup needs to happen
>> in
>>> tandem with compile-time enforcement of the rules so that we don't let
>> the
>>> bit-rott set back in again.
>>> 
>>> So really, it's about coordinating the roll-out with minimal impact on
>>> active development. Proposals?
>>> 
>>> On Tue, Nov 17, 2015 at 11:55 AM, Apekshit Sharma <appy@cloudera.com>
>>> wrote:
>>> 
>>>> Emm...do no replies mean "don't care" which means silent yes? :-)
>>>> 
>>>> On Tue, Nov 10, 2015 at 3:11 PM, Apekshit Sharma <appy@cloudera.com>
>>>> wrote:
>>>> 
>>>>> Last sunday, when i was bored, i thought  'let's fix some naive
>> stuff,
>>>>> oh.. checkstyles!'. After some time and few changes, I realized what
>>> can
>>>> be
>>>>> more boring than idling around...repeating large mechanical change
>>>> across 5
>>>>> branches :-(. And can't just do it for single branch.
>>>>> 
>>>>> Then, looking at history of checkstyle errors:
>>>>> 5th sept: 3815
>>>>> 7th sept: 1834  <-- what happened in last 2 days?
>>>>> 5th nov: 1726
>>>>> Seems like they do get fixed over time.
>>>>> 
>>>>> So the idea here is, increase the code quality bar to see how
>> good/bad
>>> we
>>>>> are, basically know where we stand, and work incrementally towards
>>>> getting
>>>>> better.
>>>>> Personally, in all my patches, i'll fix some checkstyle issues only
>>>> around
>>>>> the code am touching. But if someone wants to go on a rampage and fix
>>>> lots
>>>>> of them at once, wohooo!
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Tue, Nov 10, 2015 at 8:51 AM, Jonathan Hsieh <jon@cloudera.com>
>>>>> wrote:
>>>>> 
>>>>>> Is the goal just to incrementally fix these as we touch code or do
>> you
>>>>>> plan
>>>>>> some mass check style cleanup?
>>>>>> 
>>>>>> I've recently started contributing again and like how the checkstyle
>>>>>> scripting enforces a monotonicly decreasing # of violations.
>>>>>> 
>>>>>> On Monday, November 9, 2015, Apekshit Sharma <appy@cloudera.com>
>>> wrote:
>>>>>> 
>>>>>>> Sometimes, looking at certain things in our code makes me think,
>>> 'How
>>>>>> did
>>>>>>> it  get in, there should be some kind of check for it!'.
>>>>>>> Looked at different checkstyles rules and I think we can enable
>> the
>>>>>>> following ones to improve the formatting quality of our codebase.
>>>>>>> 
>>>>>>> ImportOrder
>>>>>>> <
>> http://checkstyle.sourceforge.net/config_imports.html#ImportOrder>
>>> :
>>>>>> keep
>>>>>>> imports in sorted order
>>>>>>> 
>>>>>>> LeftCurly <
>>>>>> http://checkstyle.sourceforge.net/config_blocks.html#LeftCurly>
>>>>>>> :
>>>>>>> Placement of left curly brace. Does 'eol' sounds right setting?
>>>>>>> 
>>>>>>> NeedBraces <
>>>>>>> http://checkstyle.sourceforge.net/config_blocks.html#NeedBraces>
>> :
>>>>>>> braces around code blocks
>>>>>>> 
>>>>>>> JavadocTagContinuationIndentation
>>>>>>> <
>> http://checkstyle.sourceforge.net/config_javadoc.html#JavadocTagContinuationIndentation
>>>>>>> :
>>>>>>> Avoid weird indentations in javadocs
>>>>>>> 
>>>>>>> NonEmptyAtclauseDescription
>>>>>>> <
>> http://checkstyle.sourceforge.net/config_javadoc.html#NonEmptyAtclauseDescription
>>>>>>> :
>>>>>>> We have sooooo many empty javadoc @ clauses. This'll take care
of
>>> it.
>>>>>>> 
>>>>>>> Indentation <
>>>>>>> http://checkstyle.sourceforge.net/config_misc.html#Indentation>
:
>>>>>>> Bad indentation hurts code readability. We have indentation
>>>> guidelines,
>>>>>>> should be fine enforcing them.
>>>>>>> 
>>>>>>> - Apekshit
>>>>>> 
>>>>>> 
>>>>>> --
>>>>>> // Jonathan Hsieh (shay)
>>>>>> // HBase Tech Lead, Software Engineer, Cloudera
>>>>>> // jon@cloudera.com // @jmhsieh
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> 
>>>>> Regards
>>>>> 
>>>>> Apekshit Sharma | Software Engineer, Cloudera | Palo Alto,
>> California |
>>>>> 650-963-6311
>>>> 
>>>> 
>>>> 
>>>> --
>>>> 
>>>> Regards
>>>> 
>>>> Apekshit Sharma | Software Engineer, Cloudera | Palo Alto, California |
>>>> 650-963-6311
>> 
>> 
>> 
>> --
>> 
>> Regards
>> 
>> Apekshit Sharma | Software Engineer, Cloudera | Palo Alto, California |
>> 650-963-6311
>> 

Mime
View raw message