Re: [DISCUSS] Code Style

[Justin Leet]

Absolutely.  I am definitely not proposing that for right away.  That's
probably a separate discuss thread anyway as we get closer to make sure we
air out any concerns and make sure we aren't leaving a lot of PRs with
difficult merges and so on.

On Tue, May 9, 2017 at 7:27 PM, Matt Foley  wrote:

> Could I ask please that any major reformat wait until the Metron 0.4.0
> release is out?
> Such across-the-board changes usually induce instability for a while, no
> matter how carefully done.
>
> Thanks,
> --Matt
> (with RM hat on)
>
> On 5/9/17, 4:12 PM, "Justin Leet"  wrote:
>
> 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 
> 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, 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
> 

Re: [DISCUSS] Code Style

[Justin Leet]

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  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, 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 
>> 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" 
>> 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 

Re: [DISCUSS] Code Style

[Justin Leet]

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  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 
> 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" 
> 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.
> > > > >>
> 

Re: [DISCUSS] Code Style

[Michael Miklavcic]

+1 Justin. Thanks.

On Mon, May 8, 2017 at 2:55 PM, Matt Foley  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"  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 
> 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 
> > 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
> > >>
> > >>
> > >
> >
>
>
>

Re: [DISCUSS] Code Style

[Matt Foley]

+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"  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  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 
> 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 
> > 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
> >>
> >>
> >
>

Re: [DISCUSS] Code Style

[Kyle Richardson]

+1 Thanks, Justin. I'm all for adopting the Google style guide.

-Kyle

On Mon, May 8, 2017 at 10:24 AM, Justin Leet  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 
> 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 
> > 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
> >>
> >>
> >
>

Re: [DISCUSS] Code Style

[Justin Leet]

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  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
>
>

Re: [DISCUSS] Code Style

[Otto Fowler]

+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

Re: [DISCUSS] Code Style

[Nick Allen]

+1 Good points, Justin.  I am onboard.



On Mon, May 8, 2017 at 9:29 AM, Justin Leet  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
>