hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Dimiduk <ndimi...@gmail.com>
Subject Re: Better code quality with more checkstyles
Date Wed, 18 Nov 2015 15:57:04 GMT
Put me dow for 6k ;)

On Tuesday, November 17, 2015, Apekshit Sharma <appy@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
>

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