crunch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Wills <jwi...@cloudera.com>
Subject Re: Checkstyle config
Date Sat, 14 Jul 2012 17:41:22 GMT
+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>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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message