I'll have a PR up a little later this week once I have a chance to sit down and test/organize notes a little more.
On Mon, May 8, 2017 at 10:04 PM, zeo...@gmail.com <zeo...@gmail.com> wrote: > +1 definitely a good idea > > On Mon, May 8, 2017, 9:07 PM Michael Miklavcic < > michael.miklav...@gmail.com> > wrote: > > > +1 Justin. Thanks. > > > > On Mon, May 8, 2017 at 2:55 PM, Matt Foley <mfo...@hortonworks.com> > wrote: > > > > > +1. I originally suggested the Sun style as a starting point, and I > find > > > Justin’s arguments convincing, especially if there is a pre-packaged > > Google > > > style checking plug-in. > > > --Matt > > > > > > On 5/8/17, 9:47 AM, "Kyle Richardson" <kylerichards...@gmail.com> > wrote: > > > > > > +1 Thanks, Justin. I'm all for adopting the Google style guide. > > > > > > -Kyle > > > > > > On Mon, May 8, 2017 at 10:24 AM, Justin Leet < > justinjl...@gmail.com> > > > wrote: > > > > > > > Sigh, the message was hidden below the fold. > > > > > > > > Part of what I've been playing around with is doing this. It's > > > pretty easy > > > > to set up both autoformatting and warnings in idea. There's a > > > checkstyle > > > > plugin, and you can just import the appropriate file on each. > As a > > > fair > > > > warning, until we do the blanket reformatting there will be a lot > > of > > > > warning highlighting in a lot of the Java files. > > > > > > > > I'll be documenting that as part of the ticket (regardless of > > > checkstyle > > > > configuration specifics), and I'll also update the wiki (or point > > to > > > links > > > > for it, I believe Checkstyle's docs walk through how to do it) as > > > part of > > > > the exercise. > > > > > > > > Sorry about spamming the list. > > > > > > > > On Mon, May 8, 2017 at 10:21 AM, Justin Leet < > > justinjl...@gmail.com> > > > > wrote: > > > > > > > > > Accidentally sent this to only Otto, instead of the whole > group. > > > > > > > > > > Part of what I've been playing around with is doing this. It's > > > pretty > > > > > easy to set up both autoformatting and warnings in idea. > > There's a > > > > > checkstyle plugin, and you can just import the appropriate file > > on > > > each. > > > > > As a fair warning, until we do the blanket reformatting there > > will > > > be a > > > > lot > > > > > of warning highlighting in a lot of the Java files. > > > > > > > > > > I'll be documenting that as part of the ticket (regardless of > > > checkstyle > > > > > configuration specifics), and I'll also update the wiki (or > point > > > to > > > > links > > > > > for it, I believe Checkstyle's docs walk through how to do it) > as > > > part of > > > > > the exercise. > > > > > > > > > > On Mon, May 8, 2017 at 9:40 AM, Otto Fowler < > > > ottobackwa...@gmail.com> > > > > > wrote: > > > > > > > > > >> +1. > > > > >> Does anyone have an idea setup for this? > > > > >> > > > > >> > > > > >> On May 8, 2017 at 09:29:15, Justin Leet ( > justinjl...@gmail.com) > > > wrote: > > > > >> > > > > >> I've been taking a look at setting up checkstyle per > > > > >> https://issues.apache.org/jira/browse/METRON-746. > > > > >> > > > > >> Given that we don't actually enforce any style right now > (saying > > > we use > > > > >> Sun's is not the same as enforcing it!), and plan to do a > > blanket > > > format > > > > >> after we get checkstyle setup, this seems like a good > > opportunity > > > to > > > > >> discuss what we want to actually implement. > > > > >> > > > > >> In particular, I'm proposing moving to Google's code style > guide > > > and > > > > away > > > > >> from Sun's. > > > > >> > > > > >> - Google's style guide already address the (admittedly) minor > > > issues we > > > > >> have with the Sun Style, in particular two spaces and line > > length > > > (100 > > > > in > > > > >> Google's). I'd prefer to just use something built-in as much > as > > > > possible, > > > > >> and it seems like Google's style is closer to what we're > looking > > > for out > > > > >> of > > > > >> the box. > > > > >> - Not only is it built in, the explanation for each rule (and > > > where > > > > >> checkstyle falls short) actually exists. > > > > >> - http://checkstyle.sourceforge.net/google_style.html > > > > >> - http://checkstyle.sourceforge.net/sun_style.html > > > > >> - Storm uses it, so it makes our code's style more compatible > > with > > > > >> one of our core dependencies (and one we've had to dig into > > > repeatedly > > > > >> during upgrades and such). > > > > >> - https://github.com/apache/storm/blob/master/pom.xml#L1100 > > > > >> - I personally find Google's guide easier to read and digest. > > I'm > > > > >> curious how other people feel. > > > > >> > > > > >> For ease of comparison: > > > > >> Sun's guide: > > http://www.oracle.com/technetwork/java/codeconvtoc- > > > 136057. > > > > >> html > > > > >> Google's guide: https://google.github.io/ > > > styleguide/javaguide.html > > > > >> > > > > >> > > > > > > > > > > > > > > > > > > > > > -- > > Jon >
