incubator-crunch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gabriel Reid <gabriel.r...@gmail.com>
Subject Re: Checkstyle config
Date Sat, 14 Jul 2012 17:45:32 GMT
Ok, sounds good, I'll go for the format-all now.

Also -- "mvn test" running in just few seconds is *awesome*, thanks Matthias!  


On Saturday 14 July 2012 at 19:41, Josh Wills wrote:

> +1. teh goog was pretty militant about style checking, and I would be happy
> to bring that process back.
>  
> I *think* we're done with major code movements as of the recent unit
> test/integration test split (which is delightful- thank you again
> Matthias.) Gabriel, it might be a good time for a format-all.
>  
> J
>  
> On Sat, Jul 14, 2012 at 6:17 AM, Gabriel Reid <gabriel.reid@gmail.com (mailto:gabriel.reid@gmail.com)>wrote:
>  
> > Hi Matthias,
> >  
> > I'm definitely in favor of going forward with this -- consistent code
> > style definitely makes it easier for me (and I think most people) to read
> > code.
> >  
> > I little while back I added an eclipse formatter profile to the project,
> > with the intention of all Eclipse users making use of it. As far as I know,
> > the formatting setup is in line with what you've outlined below, but I'll
> > have to double check it to see about getting around the "ctor def throws at
> > indentation…" warning.
> >  
> > I've been planning on doing a format-all of the whole project, but I
> > haven't done it in the past couple of weeks because there has been so much
> > file moving/renaming going on, so I didn't want to get in the way of other
> > patches at the same time.
> >  
> > - Gabriel
> >  
> >  
> > On Saturday 14 July 2012 at 11:31, Matthias Friedrich wrote:
> >  
> > > Hi,
> > >  
> > > some of you mentioned that you're interested in using Checkstyle
> > > for Crunch. I realize that people making style proposals aren't the
> > > most popular bunch, but I'll give it a try anyway ;-)
> > >  
> > > I took a first stab at a checkstyle configuration [1] that can serve
> > > as a basis for discussion. It's mostly the default config for Eclipse
> > > tweaked to fit Crunch's existing code base. Basically, I changed the
> > > following things:
> > >  
> > > * Only require Javadoc for types (classes/interfaces/enums), but
> > > eventually the entire pusblished API should be documented
> > > * Indent by 2 spaces instead of 4 (Indentation)
> > > * Set line limit from 80 to 120 characters (LineLength)
> > > * Don't allow tabs (FileTabCharacter)
> > > * Disabled DesignForExtension (we should really have this though)
> > > * Parameters may shadow fields (HiddenField)
> > > * Parameters don't need to be final (FinalParameters)
> > > * Allow magic numbers, otherwise it's too annoying (MagicNumers)
> > >  
> > > I've created a sample report [2] to give you a first impression.
> > > Checkstyle flags 900 warnings for 20000 LoC which is a pretty good
> > > value for a project that hasn't used Checkstyle during development.
> > > A relatively large number of warnings is caused by a small number of
> > > files (mostly code indented by 4 instead of 2 spaces), so there are
> > > lots of quick wins.
> > >  
> > > The 2-space indenting my prove to be difficult; I haven't been able to
> > > suppress some warnings concerning line wrapping ("ctor def throws at
> > > indentation ..."). But perhaps I've overlooked something.
> > >  
> > > What do you think? Should we continue working on this?
> > >  
> > > Regards,
> > > Matthias
> > >  
> > > [1] http://users.mafr.de/~matthias/crunch/checkstyle.xml
> > > [2] http://users.mafr.de/~matthias/crunch/checkstyle/checkstyle.html
> >  
>  
>  
>  
>  
> --  
> Director of Data Science
> Cloudera <http://www.cloudera.com>
> Twitter: @josh_wills <http://twitter.com/josh_wills>




Mime
View raw message