You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Apekshit Sharma <ap...@cloudera.com> on 2015/11/09 20:44:34 UTC
Better code quality with more checkstyles
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
Re: Better code quality with more checkstyles
Posted by Nick Dimiduk <nd...@gmail.com>.
Put me dow for 6k ;)
On Tuesday, November 17, 2015, Apekshit Sharma <ap...@cloudera.com> wrote:
> Patch is up. Tests are running. Betting is ON for what the new number will
> be. (current is ~ 3.8k) :-)
> Bets so far:
> Appy 12k
> Stack 8k
> Any other guesses?
>
> On Tue, Nov 17, 2015 at 5:29 PM, Jesse Yates <jesse.k.yates@gmail.com
> <javascript:;>>
> wrote:
>
> > +1 Sounds reasonable
> >
> > On Tue, Nov 17, 2015 at 5:25 PM Andrew Purtell <andrew.purtell@gmail.com
> <javascript:;>>
> > wrote:
> >
> > >
> > >
> > > > On Nov 17, 2015, at 5:13 PM, Stack <stack@duboce.net <javascript:;>>
> 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
> <javascript:;>>
> > > 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
> <javascript:;>>
> > > 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 <javascript:;>
> > >
> > > >>> 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 <javascript:;>
> > >
> > > >>>> 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 <javascript:;>
> > >
> > > >>>>> 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
> <javascript:;>>
> > > >>> 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 <javascript:;> // @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
> > > >>
> > >
> >
>
>
>
> --
>
> Regards
>
> Apekshit Sharma | Software Engineer, Cloudera | Palo Alto, California |
> 650-963-6311
>
Re: Better code quality with more checkstyles
Posted by Apekshit Sharma <ap...@cloudera.com>.
Patch is up. Tests are running. Betting is ON for what the new number will
be. (current is ~ 3.8k) :-)
Bets so far:
Appy 12k
Stack 8k
Any other guesses?
On Tue, Nov 17, 2015 at 5:29 PM, Jesse Yates <je...@gmail.com>
wrote:
> +1 Sounds reasonable
>
> On Tue, Nov 17, 2015 at 5:25 PM Andrew Purtell <an...@gmail.com>
> wrote:
>
> >
> >
> > > On Nov 17, 2015, at 5:13 PM, Stack <st...@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 <ap...@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 <nd...@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 <ap...@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
> > >>
> >
>
--
Regards
Apekshit Sharma | Software Engineer, Cloudera | Palo Alto, California |
650-963-6311
Re: Better code quality with more checkstyles
Posted by Jesse Yates <je...@gmail.com>.
+1 Sounds reasonable
On Tue, Nov 17, 2015 at 5:25 PM Andrew Purtell <an...@gmail.com>
wrote:
>
>
> > On Nov 17, 2015, at 5:13 PM, Stack <st...@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 <ap...@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 <nd...@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 <ap...@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 <ap...@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 <jo...@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 <ap...@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
> >>
>
Re: Better code quality with more checkstyles
Posted by Andrew Purtell <an...@gmail.com>.
> On Nov 17, 2015, at 5:13 PM, Stack <st...@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 <ap...@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 <nd...@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 <ap...@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 <ap...@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 <jo...@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 <ap...@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
>>
Re: Better code quality with more checkstyles
Posted by Stack <st...@duboce.net>.
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 <ap...@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 <nd...@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 <ap...@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 <ap...@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 <jo...@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 <ap...@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
>
Re: Better code quality with more checkstyles
Posted by Apekshit Sharma <ap...@cloudera.com>.
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 <nd...@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 <ap...@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 <ap...@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 <jo...@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 <ap...@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
Re: Better code quality with more checkstyles
Posted by Nick Dimiduk <nd...@gmail.com>.
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 <ap...@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 <ap...@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 <jo...@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 <ap...@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
>
Re: Better code quality with more checkstyles
Posted by Apekshit Sharma <ap...@cloudera.com>.
Emm...do no replies mean "don't care" which means silent yes? :-)
On Tue, Nov 10, 2015 at 3:11 PM, Apekshit Sharma <ap...@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 <jo...@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 <ap...@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
Re: Better code quality with more checkstyles
Posted by Apekshit Sharma <ap...@cloudera.com>.
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 <jo...@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 <ap...@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
Re: Better code quality with more checkstyles
Posted by Jonathan Hsieh <jo...@cloudera.com>.
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 <ap...@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