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 <[email protected] > (mailto:[email protected])>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>
