+1 Sounds reasonable On Tue, Nov 17, 2015 at 5:25 PM Andrew Purtell <andrew.purt...@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 <a...@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 <ndimi...@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 <a...@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 <a...@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 <j...@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 <a...@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 > >>>>>> // j...@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 > >> >
