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 >
