hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apekshit Sharma <a...@cloudera.com>
Subject Re: Better code quality with more checkstyles
Date Wed, 18 Nov 2015 05:54:09 GMT
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>
wrote:

> +1 Sounds reasonable
>
> On Tue, Nov 17, 2015 at 5:25 PM Andrew Purtell <andrew.purtell@gmail.com>
> wrote:
>
> >
> >
> > > 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
> > >>
> >
>



-- 

Regards

Apekshit Sharma | Software Engineer, Cloudera | Palo Alto, California |
650-963-6311

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