https://github.com/apache/incubator-metron/pull/577 is up, based on this discussion. It includes instructions for setting up both warnings in IntelliJ and autoformatting settings. They're a bit reconstructed after the fact, so if anything is wrong, let me know and I'll update them.
https://issues.apache.org/jira/browse/METRON-747 can follow more or less whenever. It's pretty easy to do a bulk reformat of only Java files. Like I said before, we probably want to regenerate anything autogenerated afterwards (i.e. anything resulting from the Stellar grammar), since I'm not convinced that they'll actually be ignored. Checkstyle supports a couple different ways of handling warnings, so the way those files use might not actually work as expected. I'd rather just blanket format, regenerated, and then call it. I am inclined to not turn on enforcement of anything until we think through exactly how we want it to work (e.g. handling just avoiding introducing new errors vs just not allowing any errors vs. setting max errors per module). I'm not sure how other projects handle this, and we probably want to look into it. On Tue, May 9, 2017 at 6:07 AM, Justin Leet <[email protected]> wrote: > 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, [email protected] <[email protected]> > wrote: > >> +1 definitely a good idea >> >> On Mon, May 8, 2017, 9:07 PM Michael Miklavcic < >> [email protected]> >> wrote: >> >> > +1 Justin. Thanks. >> > >> > On Mon, May 8, 2017 at 2:55 PM, Matt Foley <[email protected]> >> 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" <[email protected]> >> 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 < >> [email protected]> >> > > 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 < >> > [email protected]> >> > > > 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 < >> > > [email protected]> >> > > > > wrote: >> > > > > >> > > > >> +1. >> > > > >> Does anyone have an idea setup for this? >> > > > >> >> > > > >> >> > > > >> On May 8, 2017 at 09:29:15, Justin Leet ( >> [email protected]) >> > > 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 >> > >
