Re: Coding practices/standards

[Bruce Schuchardt]

I'm having trouble with Spotless.   spotlessApply on MS-Windows is 
changing all of the files it touches to have CRLF line separators. 
Adding endWithNewline() to the java config in build.gradle doesn't 
help.  If I run geode-core:spotlessApply I end up with over 5000 
modified files.



Le 10/24/2016 à 11:09 AM, Udo Kohlmeyer a écrit :

+1 - Nice work. Thx


On 24/10/16 10:40 am, Dan Smith wrote:

Doing a spotlessApply on my feature branch before rebasing didn't help
bring down the number of conflicts.

I came up with this sequence of steps to rebase a feature branch on 
develop

that avoids the need to manually resolve conflicts with the formatting
changes. The trick here is to pick up *just* the formatting changes 
in one
of the steps, and then reject any formatting changes that conflict 
with my

changes.

#Rebase onto the commit before the spotless change. Resolve conflicts 
if any

git rebase 56917a26a8916b83f0cec6e85285b5040ff66ee6

#Rebase onto the spotless change, automatically throwing away the
formatting changes if they conflict.
git rebase -Xtheirs c2319bb7a6201d5ae82ecb0fe23a1e3b8072c2e1

#Rebase onto the rest of develop. Resolve conflicts if any.
git rebase origin/develop

#Apply formatting
./gradlew geode-core:spotlessApply

-Dan

On Fri, Oct 21, 2016 at 4:34 PM, Jared Stewart  
wrote:



Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb
(from feature/spotlessPlugin).



On Oct 21, 2016, at 4:16 PM, Kirk Lund  wrote:

FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the

Apache

git repo.

Is there a way to reformat a branch and then rebase on develop to

minimize

conflicts?

-Kirk


On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart 

wrote:
Fantastic, thanks for merging this in Mark.  For anyone with 
outstanding
work on branches made before this change, your life may be made 
easier

by

cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
Spotless) into your branch and then running ‘gradlew 
spotlessApply’ on

it

before attempting to merge into develop.

— Jared
On Oct 21, 2016, at 1:33 PM, Mark Bretl  
wrote:


Thanks Jared for the suggestion of Spotless and follow-up work.

This is now completed and checked into develop. As this does 
touch many

files, be prepared the next time you pull.

--Mark



On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 

wrote:

Done! :)

- Jared

On Oct 21, 2016, at 12:27 PM, Mark Bretl 

wrote:

One more time! :)

Conflicting files
geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXDUnitTest.java

geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXWithVersionsDUnitTest.java

geode-core/src/test/java/org/apache/geode/internal/cache/execute/

PRTransactionDUnitTest.java

--Mark

On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 


wrote:

I believe there is enough consensus here to check this into

develop.
Jared, due to recent checkins into develop, can you update 
the pull

request

one more time? Trying to make this as clean as possible. I will

check

into

develop after the update, unless someone else gets to it first.

All, can we hold checkins on develop until the new formatter is

applied?

Thanks,

--Mark

On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 

wrote:

+1


On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:

+1

Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the 
formatter,

then I

am

good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 


wrote:

+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <

jstew...@pivotal.io

wrote:


I have opened a pull request here <

https://github.com/apache/
incubator-geode/pull/268> to enable the Spotless plugin 
and

to

switch to

the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund 

wrote:

For reference TRAC #38741 was a bug with the summary

"EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <

jstew...@pivotal.io

wrote:

To give everyone an update, using the Google Java Style

eclipse

template

there is an issue spotlessCheck where fails for

Re: Coding practices/standards

[Udo Kohlmeyer]

+1 - Nice work. Thx


On 24/10/16 10:40 am, Dan Smith wrote:

Doing a spotlessApply on my feature branch before rebasing didn't help
bring down the number of conflicts.

I came up with this sequence of steps to rebase a feature branch on develop
that avoids the need to manually resolve conflicts with the formatting
changes. The trick here is to pick up *just* the formatting changes in one
of the steps, and then reject any formatting changes that conflict with my
changes.

#Rebase onto the commit before the spotless change. Resolve conflicts if any
git rebase 56917a26a8916b83f0cec6e85285b5040ff66ee6

#Rebase onto the spotless change, automatically throwing away the
formatting changes if they conflict.
git rebase -Xtheirs c2319bb7a6201d5ae82ecb0fe23a1e3b8072c2e1

#Rebase onto the rest of develop. Resolve conflicts if any.
git rebase origin/develop

#Apply formatting
./gradlew geode-core:spotlessApply

-Dan

On Fri, Oct 21, 2016 at 4:34 PM, Jared Stewart  wrote:


Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb
(from feature/spotlessPlugin).



On Oct 21, 2016, at 4:16 PM, Kirk Lund  wrote:

FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the

Apache

git repo.

Is there a way to reformat a branch and then rebase on develop to

minimize

conflicts?

-Kirk


On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart 

wrote:

Fantastic, thanks for merging this in Mark.  For anyone with outstanding
work on branches made before this change, your life may be made easier

by

cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
Spotless) into your branch and then running ‘gradlew spotlessApply’ on

it

before attempting to merge into develop.

— Jared

On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:

Thanks Jared for the suggestion of Spotless and follow-up work.

This is now completed and checked into develop. As this does touch many
files, be prepared the next time you pull.

--Mark



On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 

wrote:

Done! :)

- Jared

On Oct 21, 2016, at 12:27 PM, Mark Bretl 

wrote:

One more time! :)

Conflicting files
geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXDUnitTest.java

geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXWithVersionsDUnitTest.java

geode-core/src/test/java/org/apache/geode/internal/cache/execute/

PRTransactionDUnitTest.java

--Mark

On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 

wrote:

I believe there is enough consensus here to check this into

develop.

Jared, due to recent checkins into develop, can you update the pull

request

one more time? Trying to make this as clean as possible. I will

check

into

develop after the update, unless someone else gets to it first.

All, can we hold checkins on develop until the new formatter is

applied?

Thanks,

--Mark

On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 

wrote:

+1


On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:

+1

Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the formatter,

then I

am

good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 

wrote:

+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <

jstew...@pivotal.io

wrote:


I have opened a pull request here <

https://github.com/apache/

incubator-geode/pull/268> to enable the Spotless plugin and

to

switch to

the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund 

wrote:

For reference TRAC #38741 was a bug with the summary

"EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <

jstew...@pivotal.io

wrote:

To give everyone an update, using the Google Java Style

eclipse

template

there is an issue spotlessCheck where fails for
geode-core/src/test/java/org/apache/geode/cache30/

Bug38741DUnitTest.java

even if you run it directly after spotlessApply. This needs

to

be

investigated and fixed before I can open a pull request to

enable

spotless.

On Oct 14, 2016, at 4:57 PM, Dan Smith 

Re: Coding practices/standards

[Jared Stewart]

This is awesome, thank you for taking the time to figure out how to do this 
smoothly.

—Jared
> On Oct 24, 2016, at 10:40 AM, Dan Smith  wrote:
> 
> Doing a spotlessApply on my feature branch before rebasing didn't help
> bring down the number of conflicts.
> 
> I came up with this sequence of steps to rebase a feature branch on develop
> that avoids the need to manually resolve conflicts with the formatting
> changes. The trick here is to pick up *just* the formatting changes in one
> of the steps, and then reject any formatting changes that conflict with my
> changes.
> 
> #Rebase onto the commit before the spotless change. Resolve conflicts if any
> git rebase 56917a26a8916b83f0cec6e85285b5040ff66ee6
> 
> #Rebase onto the spotless change, automatically throwing away the
> formatting changes if they conflict.
> git rebase -Xtheirs c2319bb7a6201d5ae82ecb0fe23a1e3b8072c2e1
> 
> #Rebase onto the rest of develop. Resolve conflicts if any.
> git rebase origin/develop
> 
> #Apply formatting
> ./gradlew geode-core:spotlessApply
> 
> -Dan
> 
> On Fri, Oct 21, 2016 at 4:34 PM, Jared Stewart  wrote:
> 
>> Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb
>> (from feature/spotlessPlugin).
>> 
>> 
>>> On Oct 21, 2016, at 4:16 PM, Kirk Lund  wrote:
>>> 
>>> FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the
>> Apache
>>> git repo.
>>> 
>>> Is there a way to reformat a branch and then rebase on develop to
>> minimize
>>> conflicts?
>>> 
>>> -Kirk
>>> 
>>> 
>>> On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart 
>> wrote:
>>> 
 Fantastic, thanks for merging this in Mark.  For anyone with outstanding
 work on branches made before this change, your life may be made easier
>> by
 cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
 Spotless) into your branch and then running ‘gradlew spotlessApply’ on
>> it
 before attempting to merge into develop.
 
 — Jared
> On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
> 
> Thanks Jared for the suggestion of Spotless and follow-up work.
> 
> This is now completed and checked into develop. As this does touch many
> files, be prepared the next time you pull.
> 
> --Mark
> 
> 
> 
> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
 wrote:
> 
>> Done! :)
>> 
>> - Jared
>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl 
>> wrote:
>>> 
>>> One more time! :)
>>> 
>>> Conflicting files
>>> geode-core/src/test/java/org/apache/geode/disttx/
 PRDistTXDUnitTest.java
>>> geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXWithVersionsDUnitTest.java
>>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>> PRTransactionDUnitTest.java
>>> 
>>> --Mark
>>> 
>>> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart >> 
>> wrote:
>>> 
 I just pulled and rebased onto develop, and force pushed into the
>> existing
 pull request.  It should be clean to merge in now.
 
 Thanks,
 Jared
> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
 wrote:
> 
> I believe there is enough consensus here to check this into
>> develop.
> 
> Jared, due to recent checkins into develop, can you update the pull
 request
> one more time? Trying to make this as clean as possible. I will
>> check
 into
> develop after the update, unless someone else gets to it first.
> 
> All, can we hold checkins on develop until the new formatter is
>> applied?
> 
> Thanks,
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>> wrote:
> 
>> +1
>> 
>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>> bschucha...@pivotal.io>
>> wrote:
>>> 
>>> +1
>>> 
>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
 +1
 
 
 On 20/10/16 4:56 pm, Mark Bretl wrote:
> +1 as well...
> 
> - Pulled changes
> - Executed './gradlew clean build' and was successful.
> - Modified a couple of random files to test
> - Ran './gradlew clean build' again and failed expectedly
> - Ran './gradlew spotlessApply', task was successful
> - Ran './gradlew clean build' and succeeded
> 
> Great addition! As long as others are good with the formatter,
>> then I
>> am
> good.
> 
> --Mark
> 
> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 

Re: Coding practices/standards

[Dan Smith]

Doing a spotlessApply on my feature branch before rebasing didn't help
bring down the number of conflicts.

I came up with this sequence of steps to rebase a feature branch on develop
that avoids the need to manually resolve conflicts with the formatting
changes. The trick here is to pick up *just* the formatting changes in one
of the steps, and then reject any formatting changes that conflict with my
changes.

#Rebase onto the commit before the spotless change. Resolve conflicts if any
git rebase 56917a26a8916b83f0cec6e85285b5040ff66ee6

#Rebase onto the spotless change, automatically throwing away the
formatting changes if they conflict.
git rebase -Xtheirs c2319bb7a6201d5ae82ecb0fe23a1e3b8072c2e1

#Rebase onto the rest of develop. Resolve conflicts if any.
git rebase origin/develop

#Apply formatting
./gradlew geode-core:spotlessApply

-Dan

On Fri, Oct 21, 2016 at 4:34 PM, Jared Stewart  wrote:

> Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb
> (from feature/spotlessPlugin).
>
>
> > On Oct 21, 2016, at 4:16 PM, Kirk Lund  wrote:
> >
> > FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the
> Apache
> > git repo.
> >
> > Is there a way to reformat a branch and then rebase on develop to
> minimize
> > conflicts?
> >
> > -Kirk
> >
> >
> > On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart 
> wrote:
> >
> >> Fantastic, thanks for merging this in Mark.  For anyone with outstanding
> >> work on branches made before this change, your life may be made easier
> by
> >> cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
> >> Spotless) into your branch and then running ‘gradlew spotlessApply’ on
> it
> >> before attempting to merge into develop.
> >>
> >> — Jared
> >>> On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
> >>>
> >>> Thanks Jared for the suggestion of Spotless and follow-up work.
> >>>
> >>> This is now completed and checked into develop. As this does touch many
> >>> files, be prepared the next time you pull.
> >>>
> >>> --Mark
> >>>
> >>>
> >>>
> >>> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> >> wrote:
> >>>
>  Done! :)
> 
>  - Jared
> > On Oct 21, 2016, at 12:27 PM, Mark Bretl 
> wrote:
> >
> > One more time! :)
> >
> > Conflicting files
> > geode-core/src/test/java/org/apache/geode/disttx/
> >> PRDistTXDUnitTest.java
> > geode-core/src/test/java/org/apache/geode/disttx/
>  PRDistTXWithVersionsDUnitTest.java
> > geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>  PRTransactionDUnitTest.java
> >
> > --Mark
> >
> > On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart  >
>  wrote:
> >
> >> I just pulled and rebased onto develop, and force pushed into the
>  existing
> >> pull request.  It should be clean to merge in now.
> >>
> >> Thanks,
> >> Jared
> >>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> >> wrote:
> >>>
> >>> I believe there is enough consensus here to check this into
> develop.
> >>>
> >>> Jared, due to recent checkins into develop, can you update the pull
> >> request
> >>> one more time? Trying to make this as clean as possible. I will
> check
> >> into
> >>> develop after the update, unless someone else gets to it first.
> >>>
> >>> All, can we hold checkins on develop until the new formatter is
>  applied?
> >>>
> >>> Thanks,
> >>>
> >>> --Mark
> >>>
> >>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>  wrote:
> >>>
>  +1
> 
> > On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>  bschucha...@pivotal.io>
>  wrote:
> >
> > +1
> >
> > Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> >> +1
> >>
> >>
> >> On 20/10/16 4:56 pm, Mark Bretl wrote:
> >>> +1 as well...
> >>>
> >>> - Pulled changes
> >>> - Executed './gradlew clean build' and was successful.
> >>> - Modified a couple of random files to test
> >>> - Ran './gradlew clean build' again and failed expectedly
> >>> - Ran './gradlew spotlessApply', task was successful
> >>> - Ran './gradlew clean build' and succeeded
> >>>
> >>> Great addition! As long as others are good with the formatter,
>  then I
>  am
> >>> good.
> >>>
> >>> --Mark
> >>>
> >>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
>  wrote:
> >>>
>  +1 I just added my approval to the PR (and again here)
> 
> 
>  On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
>  jstew...@pivotal.io
> >>>
>  wrote:
> 
> 

Re: Coding practices/standards

[Jared Stewart]

Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb (from 
feature/spotlessPlugin).


> On Oct 21, 2016, at 4:16 PM, Kirk Lund  wrote:
> 
> FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the Apache
> git repo.
> 
> Is there a way to reformat a branch and then rebase on develop to minimize
> conflicts?
> 
> -Kirk
> 
> 
> On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart  wrote:
> 
>> Fantastic, thanks for merging this in Mark.  For anyone with outstanding
>> work on branches made before this change, your life may be made easier by
>> cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
>> Spotless) into your branch and then running ‘gradlew spotlessApply’ on it
>> before attempting to merge into develop.
>> 
>> — Jared
>>> On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
>>> 
>>> Thanks Jared for the suggestion of Spotless and follow-up work.
>>> 
>>> This is now completed and checked into develop. As this does touch many
>>> files, be prepared the next time you pull.
>>> 
>>> --Mark
>>> 
>>> 
>>> 
>>> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
>> wrote:
>>> 
 Done! :)
 
 - Jared
> On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> 
> One more time! :)
> 
> Conflicting files
> geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXDUnitTest.java
> geode-core/src/test/java/org/apache/geode/disttx/
 PRDistTXWithVersionsDUnitTest.java
> geode-core/src/test/java/org/apache/geode/internal/cache/execute/
 PRTransactionDUnitTest.java
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
 wrote:
> 
>> I just pulled and rebased onto develop, and force pushed into the
 existing
>> pull request.  It should be clean to merge in now.
>> 
>> Thanks,
>> Jared
>>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
>> wrote:
>>> 
>>> I believe there is enough consensus here to check this into develop.
>>> 
>>> Jared, due to recent checkins into develop, can you update the pull
>> request
>>> one more time? Trying to make this as clean as possible. I will check
>> into
>>> develop after the update, unless someone else gets to it first.
>>> 
>>> All, can we hold checkins on develop until the new formatter is
 applied?
>>> 
>>> Thanks,
>>> 
>>> --Mark
>>> 
>>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
 wrote:
>>> 
 +1
 
> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
 bschucha...@pivotal.io>
 wrote:
> 
> +1
> 
> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>> +1
>> 
>> 
>> On 20/10/16 4:56 pm, Mark Bretl wrote:
>>> +1 as well...
>>> 
>>> - Pulled changes
>>> - Executed './gradlew clean build' and was successful.
>>> - Modified a couple of random files to test
>>> - Ran './gradlew clean build' again and failed expectedly
>>> - Ran './gradlew spotlessApply', task was successful
>>> - Ran './gradlew clean build' and succeeded
>>> 
>>> Great addition! As long as others are good with the formatter,
 then I
 am
>>> good.
>>> 
>>> --Mark
>>> 
>>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
 wrote:
>>> 
 +1 I just added my approval to the PR (and again here)
 
 
 On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
 jstew...@pivotal.io
>>> 
 wrote:
 
> I have opened a pull request here  incubator-geode/pull/268> to enable the Spotless plugin and to
 switch to
> the Google Java Style formatter templates.
> 
> 
>> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
 wrote:
>> 
>> For reference TRAC #38741 was a bug with the summary
 "EOFException
 during
>> deserialize on client update with copy-on-read=true"
>> 
>> -Kirk
>> 
>> 
>> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
>> jstew...@pivotal.io
> 
> wrote:
>>> To give everyone an update, using the Google Java Style
>> eclipse
 template
>>> there is an issue spotlessCheck where fails for
>>> geode-core/src/test/java/org/apache/geode/cache30/
> Bug38741DUnitTest.java
>>> even if you run it directly after spotlessApply. This needs
>> to
 be
>>> investigated and fixed before I can open a pull request to

Re: Coding practices/standards

[Kirk Lund]

FYI, feeb5c98402881156b34e222c58ce15c71a4fca7 doesn't exist in the Apache
git repo.

Is there a way to reformat a branch and then rebase on develop to minimize
conflicts?

-Kirk


On Fri, Oct 21, 2016 at 1:36 PM, Jared Stewart  wrote:

> Fantastic, thanks for merging this in Mark.  For anyone with outstanding
> work on branches made before this change, your life may be made easier by
> cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added
> Spotless) into your branch and then running ‘gradlew spotlessApply’ on it
> before attempting to merge into develop.
>
> — Jared
> > On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
> >
> > Thanks Jared for the suggestion of Spotless and follow-up work.
> >
> > This is now completed and checked into develop. As this does touch many
> > files, be prepared the next time you pull.
> >
> > --Mark
> >
> >
> >
> > On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> wrote:
> >
> >> Done! :)
> >>
> >> - Jared
> >>> On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> >>>
> >>> One more time! :)
> >>>
> >>> Conflicting files
> >>> geode-core/src/test/java/org/apache/geode/disttx/
> PRDistTXDUnitTest.java
> >>> geode-core/src/test/java/org/apache/geode/disttx/
> >> PRDistTXWithVersionsDUnitTest.java
> >>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> >> PRTransactionDUnitTest.java
> >>>
> >>> --Mark
> >>>
> >>> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> >> wrote:
> >>>
>  I just pulled and rebased onto develop, and force pushed into the
> >> existing
>  pull request.  It should be clean to merge in now.
> 
>  Thanks,
>  Jared
> > On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> wrote:
> >
> > I believe there is enough consensus here to check this into develop.
> >
> > Jared, due to recent checkins into develop, can you update the pull
>  request
> > one more time? Trying to make this as clean as possible. I will check
>  into
> > develop after the update, unless someone else gets to it first.
> >
> > All, can we hold checkins on develop until the new formatter is
> >> applied?
> >
> > Thanks,
> >
> > --Mark
> >
> > On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> >> wrote:
> >
> >> +1
> >>
> >>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> >> bschucha...@pivotal.io>
> >> wrote:
> >>>
> >>> +1
> >>>
> >>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>  +1
> 
> 
>  On 20/10/16 4:56 pm, Mark Bretl wrote:
> > +1 as well...
> >
> > - Pulled changes
> > - Executed './gradlew clean build' and was successful.
> > - Modified a couple of random files to test
> > - Ran './gradlew clean build' again and failed expectedly
> > - Ran './gradlew spotlessApply', task was successful
> > - Ran './gradlew clean build' and succeeded
> >
> > Great addition! As long as others are good with the formatter,
> >> then I
> >> am
> > good.
> >
> > --Mark
> >
> > On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> >> wrote:
> >
> >> +1 I just added my approval to the PR (and again here)
> >>
> >>
> >> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> >> jstew...@pivotal.io
> >
> >> wrote:
> >>
> >>> I have opened a pull request here  >>> incubator-geode/pull/268> to enable the Spotless plugin and to
> >> switch to
> >>> the Google Java Style formatter templates.
> >>>
> >>>
>  On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> >> wrote:
> 
>  For reference TRAC #38741 was a bug with the summary
> >> "EOFException
> >> during
>  deserialize on client update with copy-on-read=true"
> 
>  -Kirk
> 
> 
>  On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
>  jstew...@pivotal.io
> >>>
> >>> wrote:
> > To give everyone an update, using the Google Java Style
> eclipse
> >> template
> > there is an issue spotlessCheck where fails for
> > geode-core/src/test/java/org/apache/geode/cache30/
> >>> Bug38741DUnitTest.java
> > even if you run it directly after spotlessApply. This needs
> to
> >> be
> > investigated and fixed before I can open a pull request to
> >> enable
> >>> spotless.
> >
> >> On Oct 14, 2016, at 4:57 PM, Dan Smith 
>  wrote:
> >>
> >> +1 - The formatting looks better now.
> >>
> >> -Dan

Re: Coding practices/standards

[William Markito]

This looks pretty good indeed! Thanks Jared!

On Fri, Oct 21, 2016 at 3:10 PM, Dan Smith  wrote:

> We've seen the sync to github lag before when there are lots of changes,
> like when the docs were contributed. Hopefully the changes should show up
> there shortly!
>
> -Dan
>
> On Fri, Oct 21, 2016 at 2:53 PM, Jared Stewart 
> wrote:
>
> > By the way, when I pull develop from GitHub I don’t see these changes.
> > Nor do I see them when I look here  > incubator-geode/commits/develop>.  It seems that they were added to the
> > apache git repo but not the apache GitHub repo,  maybe due to the GitHub
> > DNS issues going on today?  Anyways just wanted to see if anyone else
> has a
> > better explanation.
> >
> > —Jared
> > > On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> > >
> > > I just did a pull and now I'm reviewing the formatters under etc/.
> > >
> > > -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36
> > eclipse-java-google-style.xml
> > > -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> > > eclipseOrganizeImports.importorder
> > > -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> > > intellij-java-google-style.xml
> > >
> > > What's the status of organizing imports? And do we still use
> > > eclipseOrganizeImports.importorder for imports in Eclipse?
> > >
> > > Thanks,
> > > Kirk
> > >
> > >
> > > On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl 
> > wrote:
> > >
> > >> Thanks Jared for the suggestion of Spotless and follow-up work.
> > >>
> > >> This is now completed and checked into develop. As this does touch
> many
> > >> files, be prepared the next time you pull.
> > >>
> > >> --Mark
> > >>
> > >>
> > >>
> > >> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> > >> wrote:
> > >>
> > >>> Done! :)
> > >>>
> > >>> - Jared
> >  On Oct 21, 2016, at 12:27 PM, Mark Bretl 
> > wrote:
> > 
> >  One more time! :)
> > 
> >  Conflicting files
> >  geode-core/src/test/java/org/apache/geode/disttx/
> > >> PRDistTXDUnitTest.java
> >  geode-core/src/test/java/org/apache/geode/disttx/
> > >>> PRDistTXWithVersionsDUnitTest.java
> >  geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> > >>> PRTransactionDUnitTest.java
> > 
> >  --Mark
> > 
> >  On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart <
> jstew...@pivotal.io>
> > >>> wrote:
> > 
> > > I just pulled and rebased onto develop, and force pushed into the
> > >>> existing
> > > pull request.  It should be clean to merge in now.
> > >
> > > Thanks,
> > > Jared
> > >> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> > >> wrote:
> > >>
> > >> I believe there is enough consensus here to check this into
> develop.
> > >>
> > >> Jared, due to recent checkins into develop, can you update the
> pull
> > > request
> > >> one more time? Trying to make this as clean as possible. I will
> > check
> > > into
> > >> develop after the update, unless someone else gets to it first.
> > >>
> > >> All, can we hold checkins on develop until the new formatter is
> > >>> applied?
> > >>
> > >> Thanks,
> > >>
> > >> --Mark
> > >>
> > >> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> > >>> wrote:
> > >>
> > >>> +1
> > >>>
> >  On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> > >>> bschucha...@pivotal.io>
> > >>> wrote:
> > 
> >  +1
> > 
> >  Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> > > +1
> > >
> > >
> > > On 20/10/16 4:56 pm, Mark Bretl wrote:
> > >> +1 as well...
> > >>
> > >> - Pulled changes
> > >> - Executed './gradlew clean build' and was successful.
> > >> - Modified a couple of random files to test
> > >> - Ran './gradlew clean build' again and failed expectedly
> > >> - Ran './gradlew spotlessApply', task was successful
> > >> - Ran './gradlew clean build' and succeeded
> > >>
> > >> Great addition! As long as others are good with the formatter,
> > >>> then I
> > >>> am
> > >> good.
> > >>
> > >> --Mark
> > >>
> > >> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> > >>> wrote:
> > >>
> > >>> +1 I just added my approval to the PR (and again here)
> > >>>
> > >>>
> > >>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> > >>> jstew...@pivotal.io
> > >>
> > >>> wrote:
> > >>>
> >  I have opened a pull request here <
> https://github.com/apache/
> >  incubator-geode/pull/268> to enable the Spotless plugin and
> to
> > >>> switch to
> >  the Google Java Style formatter templates.
> > 
> > 
> > 

Re: Coding practices/standards

[Dan Smith]

We've seen the sync to github lag before when there are lots of changes,
like when the docs were contributed. Hopefully the changes should show up
there shortly!

-Dan

On Fri, Oct 21, 2016 at 2:53 PM, Jared Stewart  wrote:

> By the way, when I pull develop from GitHub I don’t see these changes.
> Nor do I see them when I look here  incubator-geode/commits/develop>.  It seems that they were added to the
> apache git repo but not the apache GitHub repo,  maybe due to the GitHub
> DNS issues going on today?  Anyways just wanted to see if anyone else has a
> better explanation.
>
> —Jared
> > On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> >
> > I just did a pull and now I'm reviewing the formatters under etc/.
> >
> > -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36
> eclipse-java-google-style.xml
> > -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> > eclipseOrganizeImports.importorder
> > -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> > intellij-java-google-style.xml
> >
> > What's the status of organizing imports? And do we still use
> > eclipseOrganizeImports.importorder for imports in Eclipse?
> >
> > Thanks,
> > Kirk
> >
> >
> > On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl 
> wrote:
> >
> >> Thanks Jared for the suggestion of Spotless and follow-up work.
> >>
> >> This is now completed and checked into develop. As this does touch many
> >> files, be prepared the next time you pull.
> >>
> >> --Mark
> >>
> >>
> >>
> >> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> >> wrote:
> >>
> >>> Done! :)
> >>>
> >>> - Jared
>  On Oct 21, 2016, at 12:27 PM, Mark Bretl 
> wrote:
> 
>  One more time! :)
> 
>  Conflicting files
>  geode-core/src/test/java/org/apache/geode/disttx/
> >> PRDistTXDUnitTest.java
>  geode-core/src/test/java/org/apache/geode/disttx/
> >>> PRDistTXWithVersionsDUnitTest.java
>  geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> >>> PRTransactionDUnitTest.java
> 
>  --Mark
> 
>  On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> >>> wrote:
> 
> > I just pulled and rebased onto develop, and force pushed into the
> >>> existing
> > pull request.  It should be clean to merge in now.
> >
> > Thanks,
> > Jared
> >> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> >> wrote:
> >>
> >> I believe there is enough consensus here to check this into develop.
> >>
> >> Jared, due to recent checkins into develop, can you update the pull
> > request
> >> one more time? Trying to make this as clean as possible. I will
> check
> > into
> >> develop after the update, unless someone else gets to it first.
> >>
> >> All, can we hold checkins on develop until the new formatter is
> >>> applied?
> >>
> >> Thanks,
> >>
> >> --Mark
> >>
> >> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> >>> wrote:
> >>
> >>> +1
> >>>
>  On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> >>> bschucha...@pivotal.io>
> >>> wrote:
> 
>  +1
> 
>  Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> > +1
> >
> >
> > On 20/10/16 4:56 pm, Mark Bretl wrote:
> >> +1 as well...
> >>
> >> - Pulled changes
> >> - Executed './gradlew clean build' and was successful.
> >> - Modified a couple of random files to test
> >> - Ran './gradlew clean build' again and failed expectedly
> >> - Ran './gradlew spotlessApply', task was successful
> >> - Ran './gradlew clean build' and succeeded
> >>
> >> Great addition! As long as others are good with the formatter,
> >>> then I
> >>> am
> >> good.
> >>
> >> --Mark
> >>
> >> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> >>> wrote:
> >>
> >>> +1 I just added my approval to the PR (and again here)
> >>>
> >>>
> >>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> >>> jstew...@pivotal.io
> >>
> >>> wrote:
> >>>
>  I have opened a pull request here   incubator-geode/pull/268> to enable the Spotless plugin and to
> >>> switch to
>  the Google Java Style formatter templates.
> 
> 
> > On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> >>> wrote:
> >
> > For reference TRAC #38741 was a bug with the summary
> >>> "EOFException
> >>> during
> > deserialize on client update with copy-on-read=true"
> >
> > -Kirk
> >
> >
> > On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> > 

Re: Coding practices/standards

[Jared Stewart]

By the way, when I pull develop from GitHub I don’t see these changes.  Nor do 
I see them when I look here 
.  It seems that 
they were added to the apache git repo but not the apache GitHub repo,  maybe 
due to the GitHub DNS issues going on today?  Anyways just wanted to see if 
anyone else has a better explanation.

—Jared
> On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> 
> I just did a pull and now I'm reviewing the formatters under etc/.
> 
> -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36 eclipse-java-google-style.xml
> -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> eclipseOrganizeImports.importorder
> -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> intellij-java-google-style.xml
> 
> What's the status of organizing imports? And do we still use
> eclipseOrganizeImports.importorder for imports in Eclipse?
> 
> Thanks,
> Kirk
> 
> 
> On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl  wrote:
> 
>> Thanks Jared for the suggestion of Spotless and follow-up work.
>> 
>> This is now completed and checked into develop. As this does touch many
>> files, be prepared the next time you pull.
>> 
>> --Mark
>> 
>> 
>> 
>> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
>> wrote:
>> 
>>> Done! :)
>>> 
>>> - Jared
 On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
 
 One more time! :)
 
 Conflicting files
 geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXDUnitTest.java
 geode-core/src/test/java/org/apache/geode/disttx/
>>> PRDistTXWithVersionsDUnitTest.java
 geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>>> PRTransactionDUnitTest.java
 
 --Mark
 
 On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
>>> wrote:
 
> I just pulled and rebased onto develop, and force pushed into the
>>> existing
> pull request.  It should be clean to merge in now.
> 
> Thanks,
> Jared
>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
>> wrote:
>> 
>> I believe there is enough consensus here to check this into develop.
>> 
>> Jared, due to recent checkins into develop, can you update the pull
> request
>> one more time? Trying to make this as clean as possible. I will check
> into
>> develop after the update, unless someone else gets to it first.
>> 
>> All, can we hold checkins on develop until the new formatter is
>>> applied?
>> 
>> Thanks,
>> 
>> --Mark
>> 
>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>>> wrote:
>> 
>>> +1
>>> 
 On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>>> bschucha...@pivotal.io>
>>> wrote:
 
 +1
 
 Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> +1
> 
> 
> On 20/10/16 4:56 pm, Mark Bretl wrote:
>> +1 as well...
>> 
>> - Pulled changes
>> - Executed './gradlew clean build' and was successful.
>> - Modified a couple of random files to test
>> - Ran './gradlew clean build' again and failed expectedly
>> - Ran './gradlew spotlessApply', task was successful
>> - Ran './gradlew clean build' and succeeded
>> 
>> Great addition! As long as others are good with the formatter,
>>> then I
>>> am
>> good.
>> 
>> --Mark
>> 
>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
>>> wrote:
>> 
>>> +1 I just added my approval to the PR (and again here)
>>> 
>>> 
>>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
>>> jstew...@pivotal.io
>> 
>>> wrote:
>>> 
 I have opened a pull request here  to enable the Spotless plugin and to
>>> switch to
 the Google Java Style formatter templates.
 
 
> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
>>> wrote:
> 
> For reference TRAC #38741 was a bug with the summary
>>> "EOFException
>>> during
> deserialize on client update with copy-on-read=true"
> 
> -Kirk
> 
> 
> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> jstew...@pivotal.io
 
 wrote:
>> To give everyone an update, using the Google Java Style
>> eclipse
>>> template
>> there is an issue spotlessCheck where fails for
>> geode-core/src/test/java/org/apache/geode/cache30/
 Bug38741DUnitTest.java
>> even if you run it directly after spotlessApply. This needs
>> to
>>> be
>> investigated and fixed before I can open a pull request to
>>> enable

Re: Coding practices/standards

[Jared Stewart]

I haven’t dug in to the configuration of eclipseOrganizeImports, so for now 
spotless is not enforcing import ordering.  If we verify that the import 
ordering specified by this file looks good, I can turn that on in the spotless 
config.

— Jared
 
> On Oct 21, 2016, at 1:38 PM, Kirk Lund  wrote:
> 
> I just did a pull and now I'm reviewing the formatters under etc/.
> 
> -rw-rw-r--  1 klund  staff  36004 Oct 21 13:36 eclipse-java-google-style.xml
> -rw-rw-r--  1 klund  staff110 Sep 19 11:56
> eclipseOrganizeImports.importorder
> -rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
> intellij-java-google-style.xml
> 
> What's the status of organizing imports? And do we still use
> eclipseOrganizeImports.importorder for imports in Eclipse?
> 
> Thanks,
> Kirk
> 
> 
> On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl  wrote:
> 
>> Thanks Jared for the suggestion of Spotless and follow-up work.
>> 
>> This is now completed and checked into develop. As this does touch many
>> files, be prepared the next time you pull.
>> 
>> --Mark
>> 
>> 
>> 
>> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
>> wrote:
>> 
>>> Done! :)
>>> 
>>> - Jared
 On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
 
 One more time! :)
 
 Conflicting files
 geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXDUnitTest.java
 geode-core/src/test/java/org/apache/geode/disttx/
>>> PRDistTXWithVersionsDUnitTest.java
 geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>>> PRTransactionDUnitTest.java
 
 --Mark
 
 On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
>>> wrote:
 
> I just pulled and rebased onto develop, and force pushed into the
>>> existing
> pull request.  It should be clean to merge in now.
> 
> Thanks,
> Jared
>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
>> wrote:
>> 
>> I believe there is enough consensus here to check this into develop.
>> 
>> Jared, due to recent checkins into develop, can you update the pull
> request
>> one more time? Trying to make this as clean as possible. I will check
> into
>> develop after the update, unless someone else gets to it first.
>> 
>> All, can we hold checkins on develop until the new formatter is
>>> applied?
>> 
>> Thanks,
>> 
>> --Mark
>> 
>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>>> wrote:
>> 
>>> +1
>>> 
 On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>>> bschucha...@pivotal.io>
>>> wrote:
 
 +1
 
 Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> +1
> 
> 
> On 20/10/16 4:56 pm, Mark Bretl wrote:
>> +1 as well...
>> 
>> - Pulled changes
>> - Executed './gradlew clean build' and was successful.
>> - Modified a couple of random files to test
>> - Ran './gradlew clean build' again and failed expectedly
>> - Ran './gradlew spotlessApply', task was successful
>> - Ran './gradlew clean build' and succeeded
>> 
>> Great addition! As long as others are good with the formatter,
>>> then I
>>> am
>> good.
>> 
>> --Mark
>> 
>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
>>> wrote:
>> 
>>> +1 I just added my approval to the PR (and again here)
>>> 
>>> 
>>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
>>> jstew...@pivotal.io
>> 
>>> wrote:
>>> 
 I have opened a pull request here  to enable the Spotless plugin and to
>>> switch to
 the Google Java Style formatter templates.
 
 
> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
>>> wrote:
> 
> For reference TRAC #38741 was a bug with the summary
>>> "EOFException
>>> during
> deserialize on client update with copy-on-read=true"
> 
> -Kirk
> 
> 
> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> jstew...@pivotal.io
 
 wrote:
>> To give everyone an update, using the Google Java Style
>> eclipse
>>> template
>> there is an issue spotlessCheck where fails for
>> geode-core/src/test/java/org/apache/geode/cache30/
 Bug38741DUnitTest.java
>> even if you run it directly after spotlessApply. This needs
>> to
>>> be
>> investigated and fixed before I can open a pull request to
>>> enable
 spotless.
>> 
>>> On Oct 14, 2016, at 4:57 PM, Dan Smith 
> wrote:

Re: Coding practices/standards

[Kirk Lund]

I just did a pull and now I'm reviewing the formatters under etc/.

-rw-rw-r--  1 klund  staff  36004 Oct 21 13:36 eclipse-java-google-style.xml
-rw-rw-r--  1 klund  staff110 Sep 19 11:56
eclipseOrganizeImports.importorder
-rw-rw-r--  1 klund  staff  20653 Oct 21 13:36
intellij-java-google-style.xml

What's the status of organizing imports? And do we still use
eclipseOrganizeImports.importorder for imports in Eclipse?

Thanks,
Kirk


On Fri, Oct 21, 2016 at 1:33 PM, Mark Bretl  wrote:

> Thanks Jared for the suggestion of Spotless and follow-up work.
>
> This is now completed and checked into develop. As this does touch many
> files, be prepared the next time you pull.
>
> --Mark
>
>
>
> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart 
> wrote:
>
> > Done! :)
> >
> > - Jared
> > > On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> > >
> > > One more time! :)
> > >
> > > Conflicting files
> > > geode-core/src/test/java/org/apache/geode/disttx/
> PRDistTXDUnitTest.java
> > > geode-core/src/test/java/org/apache/geode/disttx/
> > PRDistTXWithVersionsDUnitTest.java
> > > geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> > PRTransactionDUnitTest.java
> > >
> > > --Mark
> > >
> > > On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> > wrote:
> > >
> > >> I just pulled and rebased onto develop, and force pushed into the
> > existing
> > >> pull request.  It should be clean to merge in now.
> > >>
> > >> Thanks,
> > >> Jared
> > >>> On Oct 21, 2016, at 11:57 AM, Mark Bretl 
> wrote:
> > >>>
> > >>> I believe there is enough consensus here to check this into develop.
> > >>>
> > >>> Jared, due to recent checkins into develop, can you update the pull
> > >> request
> > >>> one more time? Trying to make this as clean as possible. I will check
> > >> into
> > >>> develop after the update, unless someone else gets to it first.
> > >>>
> > >>> All, can we hold checkins on develop until the new formatter is
> > applied?
> > >>>
> > >>> Thanks,
> > >>>
> > >>> --Mark
> > >>>
> > >>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> > wrote:
> > >>>
> >  +1
> > 
> > > On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> > bschucha...@pivotal.io>
> >  wrote:
> > >
> > > +1
> > >
> > > Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> > >> +1
> > >>
> > >>
> > >> On 20/10/16 4:56 pm, Mark Bretl wrote:
> > >>> +1 as well...
> > >>>
> > >>> - Pulled changes
> > >>> - Executed './gradlew clean build' and was successful.
> > >>> - Modified a couple of random files to test
> > >>> - Ran './gradlew clean build' again and failed expectedly
> > >>> - Ran './gradlew spotlessApply', task was successful
> > >>> - Ran './gradlew clean build' and succeeded
> > >>>
> > >>> Great addition! As long as others are good with the formatter,
> > then I
> >  am
> > >>> good.
> > >>>
> > >>> --Mark
> > >>>
> > >>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> > wrote:
> > >>>
> >  +1 I just added my approval to the PR (and again here)
> > 
> > 
> >  On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> > jstew...@pivotal.io
> > >>>
> >  wrote:
> > 
> > > I have opened a pull request here  > > incubator-geode/pull/268> to enable the Spotless plugin and to
> >  switch to
> > > the Google Java Style formatter templates.
> > >
> > >
> > >> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> > wrote:
> > >>
> > >> For reference TRAC #38741 was a bug with the summary
> > "EOFException
> >  during
> > >> deserialize on client update with copy-on-read=true"
> > >>
> > >> -Kirk
> > >>
> > >>
> > >> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> > >> jstew...@pivotal.io
> > >
> > > wrote:
> > >>> To give everyone an update, using the Google Java Style
> eclipse
> >  template
> > >>> there is an issue spotlessCheck where fails for
> > >>> geode-core/src/test/java/org/apache/geode/cache30/
> > > Bug38741DUnitTest.java
> > >>> even if you run it directly after spotlessApply. This needs
> to
> > be
> > >>> investigated and fixed before I can open a pull request to
> > enable
> > > spotless.
> > >>>
> >  On Oct 14, 2016, at 4:57 PM, Dan Smith 
> > >> wrote:
> > 
> >  +1 - The formatting looks better now.
> > 
> >  -Dan
> > 
> >  On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
> >  jstew...@pivotal.io
> > >>> wrote:
> > > I agree that the formatter needs fixing up.  Our wiki <

Re: Coding practices/standards

[Udo Kohlmeyer]

+1


On 21/10/16 1:36 pm, Jared Stewart wrote:

Fantastic, thanks for merging this in Mark.  For anyone with outstanding work 
on branches made before this change, your life may be made easier by 
cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added Spotless) 
into your branch and then running ‘gradlew spotlessApply’ on it before 
attempting to merge into develop.

— Jared

On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:

Thanks Jared for the suggestion of Spotless and follow-up work.

This is now completed and checked into develop. As this does touch many
files, be prepared the next time you pull.

--Mark



On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart  wrote:


Done! :)

- Jared

On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:

One more time! :)

Conflicting files
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
geode-core/src/test/java/org/apache/geode/disttx/

PRDistTXWithVersionsDUnitTest.java

geode-core/src/test/java/org/apache/geode/internal/cache/execute/

PRTransactionDUnitTest.java

--Mark

On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 

wrote:

I just pulled and rebased onto develop, and force pushed into the

existing

pull request.  It should be clean to merge in now.

Thanks,
Jared

On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:

I believe there is enough consensus here to check this into develop.

Jared, due to recent checkins into develop, can you update the pull

request

one more time? Trying to make this as clean as possible. I will check

into

develop after the update, unless someone else gets to it first.

All, can we hold checkins on develop until the new formatter is

applied?

Thanks,

--Mark

On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 

wrote:

+1


On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <

bschucha...@pivotal.io>

wrote:

+1

Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the formatter,

then I

am

good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 

wrote:

+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <

jstew...@pivotal.io

wrote:


I have opened a pull request here  to enable the Spotless plugin and to

switch to

the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund 

wrote:

For reference TRAC #38741 was a bug with the summary

"EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <

jstew...@pivotal.io

wrote:

To give everyone an update, using the Google Java Style eclipse

template

there is an issue spotlessCheck where fails for
geode-core/src/test/java/org/apache/geode/cache30/

Bug38741DUnitTest.java

even if you run it directly after spotlessApply. This needs to

be

investigated and fixed before I can open a pull request to

enable

spotless.

On Oct 14, 2016, at 4:57 PM, Dan Smith 

wrote:

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <

jstew...@pivotal.io

wrote:

I agree that the formatter needs fixing up.  Our wiki <
https://cwiki.apache.org/confluence/display/GEODE/Code+

Style+Guide>

says

that we follow the Google Java Style guide, but that is not

actually

what’s

in our formatter templates.  I pushed a new branch <

https://github.com/

jaredjstewart/incubator-geode/tree/

spotlessPluginGoogleStyle>

that

points

spotless at the actual Google Java Style template, and I

think

it

makes

both of the examples you found look better.


On Oct 13, 2016, at 10:11 AM, Dan Smith 

wrote:

+1 for adding this to ./gradlew build

But I think we might want to fix up the formatter a bit

before

reformatting

the code. I tried running spotlessApply, and it did some

unfortunate

reformatting of code to make it less readable.

One problem is with method chaining. We have a few different

factory

APIs

that encourage method chaining, and it put all the method

calls

on

a

single

line. For example here's one change:

-ClientCacheFactory ccf = new ClientCacheFactory()
-
.addPoolServer(NetworkUtils.getServerHostName(server1.

getHost()),

port)

- .set(SECURITY_CLIENT_AUTH_INIT,
UserPasswordAuthInit.class.getName() + ".create")
-.set(SECURITY_PREFIX+"username", "root")
-.set(SECURITY_PREFIX+"password", "root");
+

Re: Coding practices/standards

[Jared Stewart]

Fantastic, thanks for merging this in Mark.  For anyone with outstanding work 
on branches made before this change, your life may be made easier by 
cherry-picking feeb5c98402881156b34e222c58ce15c71a4fca7 (which added Spotless) 
into your branch and then running ‘gradlew spotlessApply’ on it before 
attempting to merge into develop.

— Jared
> On Oct 21, 2016, at 1:33 PM, Mark Bretl  wrote:
> 
> Thanks Jared for the suggestion of Spotless and follow-up work.
> 
> This is now completed and checked into develop. As this does touch many
> files, be prepared the next time you pull.
> 
> --Mark
> 
> 
> 
> On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart  wrote:
> 
>> Done! :)
>> 
>> - Jared
>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
>>> 
>>> One more time! :)
>>> 
>>> Conflicting files
>>> geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
>>> geode-core/src/test/java/org/apache/geode/disttx/
>> PRDistTXWithVersionsDUnitTest.java
>>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/
>> PRTransactionDUnitTest.java
>>> 
>>> --Mark
>>> 
>>> On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
>> wrote:
>>> 
 I just pulled and rebased onto develop, and force pushed into the
>> existing
 pull request.  It should be clean to merge in now.
 
 Thanks,
 Jared
> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> 
> I believe there is enough consensus here to check this into develop.
> 
> Jared, due to recent checkins into develop, can you update the pull
 request
> one more time? Trying to make this as clean as possible. I will check
 into
> develop after the update, unless someone else gets to it first.
> 
> All, can we hold checkins on develop until the new formatter is
>> applied?
> 
> Thanks,
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
>> wrote:
> 
>> +1
>> 
>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
>> bschucha...@pivotal.io>
>> wrote:
>>> 
>>> +1
>>> 
>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
 +1
 
 
 On 20/10/16 4:56 pm, Mark Bretl wrote:
> +1 as well...
> 
> - Pulled changes
> - Executed './gradlew clean build' and was successful.
> - Modified a couple of random files to test
> - Ran './gradlew clean build' again and failed expectedly
> - Ran './gradlew spotlessApply', task was successful
> - Ran './gradlew clean build' and succeeded
> 
> Great addition! As long as others are good with the formatter,
>> then I
>> am
> good.
> 
> --Mark
> 
> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
>> wrote:
> 
>> +1 I just added my approval to the PR (and again here)
>> 
>> 
>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
>> jstew...@pivotal.io
> 
>> wrote:
>> 
>>> I have opened a pull request here >> incubator-geode/pull/268> to enable the Spotless plugin and to
>> switch to
>>> the Google Java Style formatter templates.
>>> 
>>> 
 On Oct 18, 2016, at 4:32 PM, Kirk Lund 
>> wrote:
 
 For reference TRAC #38741 was a bug with the summary
>> "EOFException
>> during
 deserialize on client update with copy-on-read=true"
 
 -Kirk
 
 
 On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
 jstew...@pivotal.io
>>> 
>>> wrote:
> To give everyone an update, using the Google Java Style eclipse
>> template
> there is an issue spotlessCheck where fails for
> geode-core/src/test/java/org/apache/geode/cache30/
>>> Bug38741DUnitTest.java
> even if you run it directly after spotlessApply. This needs to
>> be
> investigated and fixed before I can open a pull request to
>> enable
>>> spotless.
> 
>> On Oct 14, 2016, at 4:57 PM, Dan Smith 
 wrote:
>> 
>> +1 - The formatting looks better now.
>> 
>> -Dan
>> 
>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>> jstew...@pivotal.io
> wrote:
>>> I agree that the formatter needs fixing up.  Our wiki <
>>> https://cwiki.apache.org/confluence/display/GEODE/Code+
>> Style+Guide>
> says
>>> that we follow the Google Java Style guide, but that is not
>> actually
> what’s
>>> in our formatter templates.  I pushed a new branch <

Re: Coding practices/standards

[Mark Bretl]

Thanks Jared for the suggestion of Spotless and follow-up work.

This is now completed and checked into develop. As this does touch many
files, be prepared the next time you pull.

--Mark



On Fri, Oct 21, 2016 at 1:21 PM, Jared Stewart  wrote:

> Done! :)
>
> - Jared
> > On Oct 21, 2016, at 12:27 PM, Mark Bretl  wrote:
> >
> > One more time! :)
> >
> > Conflicting files
> > geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
> > geode-core/src/test/java/org/apache/geode/disttx/
> PRDistTXWithVersionsDUnitTest.java
> > geode-core/src/test/java/org/apache/geode/internal/cache/execute/
> PRTransactionDUnitTest.java
> >
> > --Mark
> >
> > On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart 
> wrote:
> >
> >> I just pulled and rebased onto develop, and force pushed into the
> existing
> >> pull request.  It should be clean to merge in now.
> >>
> >> Thanks,
> >> Jared
> >>> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> >>>
> >>> I believe there is enough consensus here to check this into develop.
> >>>
> >>> Jared, due to recent checkins into develop, can you update the pull
> >> request
> >>> one more time? Trying to make this as clean as possible. I will check
> >> into
> >>> develop after the update, unless someone else gets to it first.
> >>>
> >>> All, can we hold checkins on develop until the new formatter is
> applied?
> >>>
> >>> Thanks,
> >>>
> >>> --Mark
> >>>
> >>> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe 
> wrote:
> >>>
>  +1
> 
> > On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt <
> bschucha...@pivotal.io>
>  wrote:
> >
> > +1
> >
> > Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
> >> +1
> >>
> >>
> >> On 20/10/16 4:56 pm, Mark Bretl wrote:
> >>> +1 as well...
> >>>
> >>> - Pulled changes
> >>> - Executed './gradlew clean build' and was successful.
> >>> - Modified a couple of random files to test
> >>> - Ran './gradlew clean build' again and failed expectedly
> >>> - Ran './gradlew spotlessApply', task was successful
> >>> - Ran './gradlew clean build' and succeeded
> >>>
> >>> Great addition! As long as others are good with the formatter,
> then I
>  am
> >>> good.
> >>>
> >>> --Mark
> >>>
> >>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund 
> wrote:
> >>>
>  +1 I just added my approval to the PR (and again here)
> 
> 
>  On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart <
> jstew...@pivotal.io
> >>>
>  wrote:
> 
> > I have opened a pull request here  > incubator-geode/pull/268> to enable the Spotless plugin and to
>  switch to
> > the Google Java Style formatter templates.
> >
> >
> >> On Oct 18, 2016, at 4:32 PM, Kirk Lund 
> wrote:
> >>
> >> For reference TRAC #38741 was a bug with the summary
> "EOFException
>  during
> >> deserialize on client update with copy-on-read=true"
> >>
> >> -Kirk
> >>
> >>
> >> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> >> jstew...@pivotal.io
> >
> > wrote:
> >>> To give everyone an update, using the Google Java Style eclipse
>  template
> >>> there is an issue spotlessCheck where fails for
> >>> geode-core/src/test/java/org/apache/geode/cache30/
> > Bug38741DUnitTest.java
> >>> even if you run it directly after spotlessApply. This needs to
> be
> >>> investigated and fixed before I can open a pull request to
> enable
> > spotless.
> >>>
>  On Oct 14, 2016, at 4:57 PM, Dan Smith 
> >> wrote:
> 
>  +1 - The formatting looks better now.
> 
>  -Dan
> 
>  On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>  jstew...@pivotal.io
> >>> wrote:
> > I agree that the formatter needs fixing up.  Our wiki <
> > https://cwiki.apache.org/confluence/display/GEODE/Code+
>  Style+Guide>
> >>> says
> > that we follow the Google Java Style guide, but that is not
>  actually
> >>> what’s
> > in our formatter templates.  I pushed a new branch <
> > https://github.com/
> > jaredjstewart/incubator-geode/tree/
> spotlessPluginGoogleStyle>
>  that
> >>> points
> > spotless at the actual Google Java Style template, and I
> think
> >> it
> > makes
> > both of the examples you found look better.
> >
> >> On Oct 13, 2016, at 10:11 AM, Dan Smith 
>  wrote:
> >>
> >> +1 for adding this to ./gradlew build
> 

Re: Coding practices/standards

[Mark Bretl]

One more time! :)

Conflicting files
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXDUnitTest.java
geode-core/src/test/java/org/apache/geode/disttx/PRDistTXWithVersionsDUnitTest.java
geode-core/src/test/java/org/apache/geode/internal/cache/execute/PRTransactionDUnitTest.java

--Mark

On Fri, Oct 21, 2016 at 12:08 PM, Jared Stewart  wrote:

> I just pulled and rebased onto develop, and force pushed into the existing
> pull request.  It should be clean to merge in now.
>
> Thanks,
> Jared
> > On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> >
> > I believe there is enough consensus here to check this into develop.
> >
> > Jared, due to recent checkins into develop, can you update the pull
> request
> > one more time? Trying to make this as clean as possible. I will check
> into
> > develop after the update, unless someone else gets to it first.
> >
> > All, can we hold checkins on develop until the new formatter is applied?
> >
> > Thanks,
> >
> > --Mark
> >
> > On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe  wrote:
> >
> >> +1
> >>
> >>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt 
> >> wrote:
> >>>
> >>> +1
> >>>
> >>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>  +1
> 
> 
>  On 20/10/16 4:56 pm, Mark Bretl wrote:
> > +1 as well...
> >
> > - Pulled changes
> > - Executed './gradlew clean build' and was successful.
> > - Modified a couple of random files to test
> > - Ran './gradlew clean build' again and failed expectedly
> > - Ran './gradlew spotlessApply', task was successful
> > - Ran './gradlew clean build' and succeeded
> >
> > Great addition! As long as others are good with the formatter, then I
> >> am
> > good.
> >
> > --Mark
> >
> > On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
> >
> >> +1 I just added my approval to the PR (and again here)
> >>
> >>
> >> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart  >
> >> wrote:
> >>
> >>> I have opened a pull request here  >>> incubator-geode/pull/268> to enable the Spotless plugin and to
> >> switch to
> >>> the Google Java Style formatter templates.
> >>>
> >>>
>  On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
> 
>  For reference TRAC #38741 was a bug with the summary "EOFException
> >> during
>  deserialize on client update with copy-on-read=true"
> 
>  -Kirk
> 
> 
>  On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart <
> jstew...@pivotal.io
> >>>
> >>> wrote:
> > To give everyone an update, using the Google Java Style eclipse
> >> template
> > there is an issue spotlessCheck where fails for
> > geode-core/src/test/java/org/apache/geode/cache30/
> >>> Bug38741DUnitTest.java
> > even if you run it directly after spotlessApply. This needs to be
> > investigated and fixed before I can open a pull request to enable
> >>> spotless.
> >
> >> On Oct 14, 2016, at 4:57 PM, Dan Smith 
> wrote:
> >>
> >> +1 - The formatting looks better now.
> >>
> >> -Dan
> >>
> >> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
> >> jstew...@pivotal.io
> > wrote:
> >>> I agree that the formatter needs fixing up.  Our wiki <
> >>> https://cwiki.apache.org/confluence/display/GEODE/Code+
> >> Style+Guide>
> > says
> >>> that we follow the Google Java Style guide, but that is not
> >> actually
> > what’s
> >>> in our formatter templates.  I pushed a new branch <
> >>> https://github.com/
> >>> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle>
> >> that
> > points
> >>> spotless at the actual Google Java Style template, and I think
> it
> >>> makes
> >>> both of the examples you found look better.
> >>>
>  On Oct 13, 2016, at 10:11 AM, Dan Smith 
> >> wrote:
> 
>  +1 for adding this to ./gradlew build
> 
>  But I think we might want to fix up the formatter a bit before
> >>> reformatting
>  the code. I tried running spotlessApply, and it did some
> >> unfortunate
>  reformatting of code to make it less readable.
> 
>  One problem is with method chaining. We have a few different
> >> factory
> > APIs
>  that encourage method chaining, and it put all the method
> calls
> >> on
> >> a
> >>> single
>  line. For example here's one change:
> 
>  -ClientCacheFactory ccf = new ClientCacheFactory()
>  -
>  

Re: Coding practices/standards

[Jared Stewart]

I just pulled and rebased onto develop, and force pushed into the existing pull 
request.  It should be clean to merge in now.

Thanks,
Jared
> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> 
> I believe there is enough consensus here to check this into develop.
> 
> Jared, due to recent checkins into develop, can you update the pull request
> one more time? Trying to make this as clean as possible. I will check into
> develop after the update, unless someone else gets to it first.
> 
> All, can we hold checkins on develop until the new formatter is applied?
> 
> Thanks,
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe  wrote:
> 
>> +1
>> 
>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> +1
>>> 
>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
 +1
 
 
 On 20/10/16 4:56 pm, Mark Bretl wrote:
> +1 as well...
> 
> - Pulled changes
> - Executed './gradlew clean build' and was successful.
> - Modified a couple of random files to test
> - Ran './gradlew clean build' again and failed expectedly
> - Ran './gradlew spotlessApply', task was successful
> - Ran './gradlew clean build' and succeeded
> 
> Great addition! As long as others are good with the formatter, then I
>> am
> good.
> 
> --Mark
> 
> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
> 
>> +1 I just added my approval to the PR (and again here)
>> 
>> 
>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
>> wrote:
>> 
>>> I have opened a pull request here >> incubator-geode/pull/268> to enable the Spotless plugin and to
>> switch to
>>> the Google Java Style formatter templates.
>>> 
>>> 
 On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
 
 For reference TRAC #38741 was a bug with the summary "EOFException
>> during
 deserialize on client update with copy-on-read=true"
 
 -Kirk
 
 
 On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart >> 
>>> wrote:
> To give everyone an update, using the Google Java Style eclipse
>> template
> there is an issue spotlessCheck where fails for
> geode-core/src/test/java/org/apache/geode/cache30/
>>> Bug38741DUnitTest.java
> even if you run it directly after spotlessApply. This needs to be
> investigated and fixed before I can open a pull request to enable
>>> spotless.
> 
>> On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
>> 
>> +1 - The formatting looks better now.
>> 
>> -Dan
>> 
>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>> jstew...@pivotal.io
> wrote:
>>> I agree that the formatter needs fixing up.  Our wiki <
>>> https://cwiki.apache.org/confluence/display/GEODE/Code+
>> Style+Guide>
> says
>>> that we follow the Google Java Style guide, but that is not
>> actually
> what’s
>>> in our formatter templates.  I pushed a new branch <
>>> https://github.com/
>>> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle>
>> that
> points
>>> spotless at the actual Google Java Style template, and I think it
>>> makes
>>> both of the examples you found look better.
>>> 
 On Oct 13, 2016, at 10:11 AM, Dan Smith 
>> wrote:
 
 +1 for adding this to ./gradlew build
 
 But I think we might want to fix up the formatter a bit before
>>> reformatting
 the code. I tried running spotlessApply, and it did some
>> unfortunate
 reformatting of code to make it less readable.
 
 One problem is with method chaining. We have a few different
>> factory
> APIs
 that encourage method chaining, and it put all the method calls
>> on
>> a
>>> single
 line. For example here's one change:
 
 -ClientCacheFactory ccf = new ClientCacheFactory()
 -
 .addPoolServer(NetworkUtils.getServerHostName(server1.
>> getHost()),
> port)
 - .set(SECURITY_CLIENT_AUTH_INIT,
 UserPasswordAuthInit.class.getName() + ".create")
 -.set(SECURITY_PREFIX+"username", "root")
 -.set(SECURITY_PREFIX+"password", "root");
 +ClientCacheFactory ccf = new
 ClientCacheFactory().addPoolServer(NetworkUtils.
>>> getServerHostName(server1.getHost()),
 port).set(SECURITY_CLIENT_AUTH_INIT,
>> UserPasswordAuthInit.class.
> getName()
>>> +
 

Re: Coding practices/standards

[Anthony Baker]

Sounds like a plan!  Let’s do this :-)

Anthony

> On Oct 21, 2016, at 11:57 AM, Mark Bretl  wrote:
> 
> I believe there is enough consensus here to check this into develop.
> 
> Jared, due to recent checkins into develop, can you update the pull request
> one more time? Trying to make this as clean as possible. I will check into
> develop after the update, unless someone else gets to it first.
> 
> All, can we hold checkins on develop until the new formatter is applied?
> 
> Thanks,
> 
> --Mark
> 
> On Fri, Oct 21, 2016 at 9:03 AM, Kenneth Howe  wrote:
> 
>> +1
>> 
>>> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> +1
>>> 
>>> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
 +1
 
 
 On 20/10/16 4:56 pm, Mark Bretl wrote:
> +1 as well...
> 
> - Pulled changes
> - Executed './gradlew clean build' and was successful.
> - Modified a couple of random files to test
> - Ran './gradlew clean build' again and failed expectedly
> - Ran './gradlew spotlessApply', task was successful
> - Ran './gradlew clean build' and succeeded
> 
> Great addition! As long as others are good with the formatter, then I
>> am
> good.
> 
> --Mark
> 
> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
> 
>> +1 I just added my approval to the PR (and again here)
>> 
>> 
>> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
>> wrote:
>> 
>>> I have opened a pull request here >> incubator-geode/pull/268> to enable the Spotless plugin and to
>> switch to
>>> the Google Java Style formatter templates.
>>> 
>>> 
 On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
 
 For reference TRAC #38741 was a bug with the summary "EOFException
>> during
 deserialize on client update with copy-on-read=true"
 
 -Kirk
 
 
 On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart >> 
>>> wrote:
> To give everyone an update, using the Google Java Style eclipse
>> template
> there is an issue spotlessCheck where fails for
> geode-core/src/test/java/org/apache/geode/cache30/
>>> Bug38741DUnitTest.java
> even if you run it directly after spotlessApply. This needs to be
> investigated and fixed before I can open a pull request to enable
>>> spotless.
> 
>> On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
>> 
>> +1 - The formatting looks better now.
>> 
>> -Dan
>> 
>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart <
>> jstew...@pivotal.io
> wrote:
>>> I agree that the formatter needs fixing up.  Our wiki <
>>> https://cwiki.apache.org/confluence/display/GEODE/Code+
>> Style+Guide>
> says
>>> that we follow the Google Java Style guide, but that is not
>> actually
> what’s
>>> in our formatter templates.  I pushed a new branch <
>>> https://github.com/
>>> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle>
>> that
> points
>>> spotless at the actual Google Java Style template, and I think it
>>> makes
>>> both of the examples you found look better.
>>> 
 On Oct 13, 2016, at 10:11 AM, Dan Smith 
>> wrote:
 
 +1 for adding this to ./gradlew build
 
 But I think we might want to fix up the formatter a bit before
>>> reformatting
 the code. I tried running spotlessApply, and it did some
>> unfortunate
 reformatting of code to make it less readable.
 
 One problem is with method chaining. We have a few different
>> factory
> APIs
 that encourage method chaining, and it put all the method calls
>> on
>> a
>>> single
 line. For example here's one change:
 
 -ClientCacheFactory ccf = new ClientCacheFactory()
 -
 .addPoolServer(NetworkUtils.getServerHostName(server1.
>> getHost()),
> port)
 - .set(SECURITY_CLIENT_AUTH_INIT,
 UserPasswordAuthInit.class.getName() + ".create")
 -.set(SECURITY_PREFIX+"username", "root")
 -.set(SECURITY_PREFIX+"password", "root");
 +ClientCacheFactory ccf = new
 ClientCacheFactory().addPoolServer(NetworkUtils.
>>> getServerHostName(server1.getHost()),
 port).set(SECURITY_CLIENT_AUTH_INIT,
>> UserPasswordAuthInit.class.
> getName()
>>> +
 ".create").set(SECURITY_PREFIX + "username",
> "root").set(SECURITY_PREFIX
>>> +

Re: Coding practices/standards

[Kenneth Howe]

+1

> On Oct 21, 2016, at 8:27 AM, Bruce Schuchardt  wrote:
> 
> +1
> 
> Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :
>> +1
>> 
>> 
>> On 20/10/16 4:56 pm, Mark Bretl wrote:
>>> +1 as well...
>>> 
>>> - Pulled changes
>>> - Executed './gradlew clean build' and was successful.
>>> - Modified a couple of random files to test
>>> - Ran './gradlew clean build' again and failed expectedly
>>> - Ran './gradlew spotlessApply', task was successful
>>> - Ran './gradlew clean build' and succeeded
>>> 
>>> Great addition! As long as others are good with the formatter, then I am
>>> good.
>>> 
>>> --Mark
>>> 
>>> On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:
>>> 
 +1 I just added my approval to the PR (and again here)
 
 
 On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
 wrote:
 
> I have opened a pull request here  incubator-geode/pull/268> to enable the Spotless plugin and to switch to
> the Google Java Style formatter templates.
> 
> 
>> On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
>> 
>> For reference TRAC #38741 was a bug with the summary "EOFException
 during
>> deserialize on client update with copy-on-read=true"
>> 
>> -Kirk
>> 
>> 
>> On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart 
> wrote:
>>> To give everyone an update, using the Google Java Style eclipse
 template
>>> there is an issue spotlessCheck where fails for
>>> geode-core/src/test/java/org/apache/geode/cache30/
> Bug38741DUnitTest.java
>>> even if you run it directly after spotlessApply. This needs to be
>>> investigated and fixed before I can open a pull request to enable
> spotless.
>>> 
 On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
 
 +1 - The formatting looks better now.
 
 -Dan
 
 On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart >> wrote:
> I agree that the formatter needs fixing up.  Our wiki <
> https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide> 
>>> says
> that we follow the Google Java Style guide, but that is not actually
>>> what’s
> in our formatter templates.  I pushed a new branch <
> https://github.com/
> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that
>>> points
> spotless at the actual Google Java Style template, and I think it
> makes
> both of the examples you found look better.
> 
>> On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
>> 
>> +1 for adding this to ./gradlew build
>> 
>> But I think we might want to fix up the formatter a bit before
> reformatting
>> the code. I tried running spotlessApply, and it did some
 unfortunate
>> reformatting of code to make it less readable.
>> 
>> One problem is with method chaining. We have a few different
 factory
>>> APIs
>> that encourage method chaining, and it put all the method calls on
 a
> single
>> line. For example here's one change:
>> 
>> -ClientCacheFactory ccf = new ClientCacheFactory()
>> -
>> .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),
>>> port)
>> - .set(SECURITY_CLIENT_AUTH_INIT,
>> UserPasswordAuthInit.class.getName() + ".create")
>> -.set(SECURITY_PREFIX+"username", "root")
>> -.set(SECURITY_PREFIX+"password", "root");
>> +ClientCacheFactory ccf = new
>> ClientCacheFactory().addPoolServer(NetworkUtils.
> getServerHostName(server1.getHost()),
>> port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.
>>> getName()
> +
>> ".create").set(SECURITY_PREFIX + "username",
>>> "root").set(SECURITY_PREFIX
> +
>> "password", "root");
>> 
>> I see a similar problem where it put array initialization all on a
>>> single
>> line:
>> 
>> +  public void testMultiColOrderByWithIndexResultWithProjection()
>>> throws
>> Exception {
>>   String queries[] = {
>>   // Test case No. IUMR021
>> -"SELECT   ID, description, createTime, pkid FROM
 /portfolio1
>>> pf1
>> where ID > 10 order by ID desc, pkid desc ",
>> -"SELECT   ID, description, createTime, pkid FROM
 /portfolio1
>>> pf1
>> where ID > 10 order by ID asc, pkid asc ",
>> -"SELECT   ID, description, createTime, pkid FROM
 /portfolio1
>>> pf1
>> where ID > 10 and ID < 20 order by ID asc, pkid asc ",
>> -"SELECT   ID, 

Re: Coding practices/standards

[Bruce Schuchardt]

+1

Le 10/20/2016 à 5:13 PM, Udo Kohlmeyer a écrit :

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the formatter, then I am
good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:


+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
wrote:


I have opened a pull request here  to enable the Spotless plugin and to 
switch to

the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:

For reference TRAC #38741 was a bug with the summary "EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart 

wrote:

To give everyone an update, using the Google Java Style eclipse

template

there is an issue spotlessCheck where fails for
geode-core/src/test/java/org/apache/geode/cache30/

Bug38741DUnitTest.java

even if you run it directly after spotlessApply. This needs to be
investigated and fixed before I can open a pull request to enable

spotless.



On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart 
 


says
that we follow the Google Java Style guide, but that is not 
actually

what’s

in our formatter templates.  I pushed a new branch <

https://github.com/

jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that

points

spotless at the actual Google Java Style template, and I think it

makes

both of the examples you found look better.

On Oct 13, 2016, at 10:11 AM, Dan Smith  
wrote:


+1 for adding this to ./gradlew build

But I think we might want to fix up the formatter a bit before

reformatting

the code. I tried running spotlessApply, and it did some

unfortunate

reformatting of code to make it less readable.

One problem is with method chaining. We have a few different

factory

APIs
that encourage method chaining, and it put all the method 
calls on

a

single

line. For example here's one change:

-ClientCacheFactory ccf = new ClientCacheFactory()
-
.addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),

port)

- .set(SECURITY_CLIENT_AUTH_INIT,
UserPasswordAuthInit.class.getName() + ".create")
-.set(SECURITY_PREFIX+"username", "root")
-.set(SECURITY_PREFIX+"password", "root");
+ClientCacheFactory ccf = new
ClientCacheFactory().addPoolServer(NetworkUtils.

getServerHostName(server1.getHost()),

port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.

getName()

+

".create").set(SECURITY_PREFIX + "username",

"root").set(SECURITY_PREFIX

+

"password", "root");

I see a similar problem where it put array initialization all 
on a

single

line:

+  public void testMultiColOrderByWithIndexResultWithProjection()

throws

Exception {
   String queries[] = {
   // Test case No. IUMR021
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID desc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID asc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID asc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
-"SELECT   ID, description, createTime, pkid 

Re: Coding practices/standards

[Udo Kohlmeyer]

+1


On 20/10/16 4:56 pm, Mark Bretl wrote:

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the formatter, then I am
good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:


+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
wrote:


I have opened a pull request here  to enable the Spotless plugin and to switch to
the Google Java Style formatter templates.



On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:

For reference TRAC #38741 was a bug with the summary "EOFException

during

deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart 

wrote:

To give everyone an update, using the Google Java Style eclipse

template

there is an issue spotlessCheck where fails for
geode-core/src/test/java/org/apache/geode/cache30/

Bug38741DUnitTest.java

even if you run it directly after spotlessApply. This needs to be
investigated and fixed before I can open a pull request to enable

spotless.



On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart 

says

that we follow the Google Java Style guide, but that is not actually

what’s

in our formatter templates.  I pushed a new branch <

https://github.com/

jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that

points

spotless at the actual Google Java Style template, and I think it

makes

both of the examples you found look better.


On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:

+1 for adding this to ./gradlew build

But I think we might want to fix up the formatter a bit before

reformatting

the code. I tried running spotlessApply, and it did some

unfortunate

reformatting of code to make it less readable.

One problem is with method chaining. We have a few different

factory

APIs

that encourage method chaining, and it put all the method calls on

a

single

line. For example here's one change:

-ClientCacheFactory ccf = new ClientCacheFactory()
-
.addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),

port)

-.set(SECURITY_CLIENT_AUTH_INIT,
UserPasswordAuthInit.class.getName() + ".create")
-.set(SECURITY_PREFIX+"username", "root")
-.set(SECURITY_PREFIX+"password", "root");
+ClientCacheFactory ccf = new
ClientCacheFactory().addPoolServer(NetworkUtils.

getServerHostName(server1.getHost()),

port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.

getName()

+

".create").set(SECURITY_PREFIX + "username",

"root").set(SECURITY_PREFIX

+

"password", "root");

I see a similar problem where it put array initialization all on a

single

line:

+  public void testMultiColOrderByWithIndexResultWithProjection()

throws

Exception {
   String queries[] = {
   // Test case No. IUMR021
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID desc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID asc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID != 10 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID desc, pkid desc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 order by ID asc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
-"SELECT   ID, description, createTime, pkid FROM

/portfolio1

pf1

where ID > 10 and ID < 20 

Re: Coding practices/standards

[Mark Bretl]

+1 as well...

- Pulled changes
- Executed './gradlew clean build' and was successful.
- Modified a couple of random files to test
- Ran './gradlew clean build' again and failed expectedly
- Ran './gradlew spotlessApply', task was successful
- Ran './gradlew clean build' and succeeded

Great addition! As long as others are good with the formatter, then I am
good.

--Mark

On Thu, Oct 20, 2016 at 3:40 PM, Kirk Lund  wrote:

> +1 I just added my approval to the PR (and again here)
>
>
> On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart 
> wrote:
>
> > I have opened a pull request here  > incubator-geode/pull/268> to enable the Spotless plugin and to switch to
> > the Google Java Style formatter templates.
> >
> >
> > > On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
> > >
> > > For reference TRAC #38741 was a bug with the summary "EOFException
> during
> > > deserialize on client update with copy-on-read=true"
> > >
> > > -Kirk
> > >
> > >
> > > On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart 
> > wrote:
> > >
> > >> To give everyone an update, using the Google Java Style eclipse
> template
> > >> there is an issue spotlessCheck where fails for
> > >> geode-core/src/test/java/org/apache/geode/cache30/
> > Bug38741DUnitTest.java
> > >> even if you run it directly after spotlessApply. This needs to be
> > >> investigated and fixed before I can open a pull request to enable
> > spotless.
> > >>
> > >>
> > >>> On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
> > >>>
> > >>> +1 - The formatting looks better now.
> > >>>
> > >>> -Dan
> > >>>
> > >>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart  >
> > >> wrote:
> > >>>
> >  I agree that the formatter needs fixing up.  Our wiki <
> >  https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide>
> > >> says
> >  that we follow the Google Java Style guide, but that is not actually
> > >> what’s
> >  in our formatter templates.  I pushed a new branch <
> > https://github.com/
> >  jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that
> > >> points
> >  spotless at the actual Google Java Style template, and I think it
> > makes
> >  both of the examples you found look better.
> > 
> > > On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
> > >
> > > +1 for adding this to ./gradlew build
> > >
> > > But I think we might want to fix up the formatter a bit before
> >  reformatting
> > > the code. I tried running spotlessApply, and it did some
> unfortunate
> > > reformatting of code to make it less readable.
> > >
> > > One problem is with method chaining. We have a few different
> factory
> > >> APIs
> > > that encourage method chaining, and it put all the method calls on
> a
> >  single
> > > line. For example here's one change:
> > >
> > > -ClientCacheFactory ccf = new ClientCacheFactory()
> > > -
> > > .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),
> > >> port)
> > > -.set(SECURITY_CLIENT_AUTH_INIT,
> > > UserPasswordAuthInit.class.getName() + ".create")
> > > -.set(SECURITY_PREFIX+"username", "root")
> > > -.set(SECURITY_PREFIX+"password", "root");
> > > +ClientCacheFactory ccf = new
> > > ClientCacheFactory().addPoolServer(NetworkUtils.
> >  getServerHostName(server1.getHost()),
> > > port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.
> > >> getName()
> >  +
> > > ".create").set(SECURITY_PREFIX + "username",
> > >> "root").set(SECURITY_PREFIX
> >  +
> > > "password", "root");
> > >
> > > I see a similar problem where it put array initialization all on a
> > >> single
> > > line:
> > >
> > > +  public void testMultiColOrderByWithIndexResultWithProjection()
> > >> throws
> > > Exception {
> > >   String queries[] = {
> > >   // Test case No. IUMR021
> > > -"SELECT   ID, description, createTime, pkid FROM
> /portfolio1
> > >> pf1
> > > where ID > 10 order by ID desc, pkid desc ",
> > > -"SELECT   ID, description, createTime, pkid FROM
> /portfolio1
> > >> pf1
> > > where ID > 10 order by ID asc, pkid asc ",
> > > -"SELECT   ID, description, createTime, pkid FROM
> /portfolio1
> > >> pf1
> > > where ID > 10 and ID < 20 order by ID asc, pkid asc ",
> > > -"SELECT   ID, description, createTime, pkid FROM
> /portfolio1
> > >> pf1
> > > where ID > 10 and ID < 20 order by ID desc , pkid desc",
> > > -"SELECT   ID, description, createTime, pkid FROM
> /portfolio1
> > >> pf1
> > > where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
> > > -"SELECT   ID, description, createTime, pkid FROM
> /portfolio1
> > >> pf1
> > > where ID >= 10 and 

Re: Coding practices/standards

[Kirk Lund]

+1 I just added my approval to the PR (and again here)


On Thu, Oct 20, 2016 at 3:25 PM, Jared Stewart  wrote:

> I have opened a pull request here  incubator-geode/pull/268> to enable the Spotless plugin and to switch to
> the Google Java Style formatter templates.
>
>
> > On Oct 18, 2016, at 4:32 PM, Kirk Lund  wrote:
> >
> > For reference TRAC #38741 was a bug with the summary "EOFException during
> > deserialize on client update with copy-on-read=true"
> >
> > -Kirk
> >
> >
> > On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart 
> wrote:
> >
> >> To give everyone an update, using the Google Java Style eclipse template
> >> there is an issue spotlessCheck where fails for
> >> geode-core/src/test/java/org/apache/geode/cache30/
> Bug38741DUnitTest.java
> >> even if you run it directly after spotlessApply. This needs to be
> >> investigated and fixed before I can open a pull request to enable
> spotless.
> >>
> >>
> >>> On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
> >>>
> >>> +1 - The formatting looks better now.
> >>>
> >>> -Dan
> >>>
> >>> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart 
> >> wrote:
> >>>
>  I agree that the formatter needs fixing up.  Our wiki <
>  https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide>
> >> says
>  that we follow the Google Java Style guide, but that is not actually
> >> what’s
>  in our formatter templates.  I pushed a new branch <
> https://github.com/
>  jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that
> >> points
>  spotless at the actual Google Java Style template, and I think it
> makes
>  both of the examples you found look better.
> 
> > On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
> >
> > +1 for adding this to ./gradlew build
> >
> > But I think we might want to fix up the formatter a bit before
>  reformatting
> > the code. I tried running spotlessApply, and it did some unfortunate
> > reformatting of code to make it less readable.
> >
> > One problem is with method chaining. We have a few different factory
> >> APIs
> > that encourage method chaining, and it put all the method calls on a
>  single
> > line. For example here's one change:
> >
> > -ClientCacheFactory ccf = new ClientCacheFactory()
> > -
> > .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),
> >> port)
> > -.set(SECURITY_CLIENT_AUTH_INIT,
> > UserPasswordAuthInit.class.getName() + ".create")
> > -.set(SECURITY_PREFIX+"username", "root")
> > -.set(SECURITY_PREFIX+"password", "root");
> > +ClientCacheFactory ccf = new
> > ClientCacheFactory().addPoolServer(NetworkUtils.
>  getServerHostName(server1.getHost()),
> > port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.
> >> getName()
>  +
> > ".create").set(SECURITY_PREFIX + "username",
> >> "root").set(SECURITY_PREFIX
>  +
> > "password", "root");
> >
> > I see a similar problem where it put array initialization all on a
> >> single
> > line:
> >
> > +  public void testMultiColOrderByWithIndexResultWithProjection()
> >> throws
> > Exception {
> >   String queries[] = {
> >   // Test case No. IUMR021
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID > 10 order by ID desc, pkid desc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID > 10 order by ID asc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID > 10 and ID < 20 order by ID asc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID > 10 and ID < 20 order by ID desc , pkid desc",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID != 10 order by ID asc , pkid desc",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID != 10 order by ID desc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID > 10 order by ID desc, pkid desc limit 5",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1
> > where ID > 10 order by ID asc, pkid asc limit 5",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> >> pf1

Re: Coding practices/standards

[Kirk Lund]

For reference TRAC #38741 was a bug with the summary "EOFException during
deserialize on client update with copy-on-read=true"

-Kirk


On Tue, Oct 18, 2016 at 4:27 PM, Jared Stewart  wrote:

> To give everyone an update, using the Google Java Style eclipse template
> there is an issue spotlessCheck where fails for
> geode-core/src/test/java/org/apache/geode/cache30/Bug38741DUnitTest.java
> even if you run it directly after spotlessApply. This needs to be
> investigated and fixed before I can open a pull request to enable spotless.
>
>
> > On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
> >
> > +1 - The formatting looks better now.
> >
> > -Dan
> >
> > On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart 
> wrote:
> >
> >> I agree that the formatter needs fixing up.  Our wiki <
> >> https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide>
> says
> >> that we follow the Google Java Style guide, but that is not actually
> what’s
> >> in our formatter templates.  I pushed a new branch  >> jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that
> points
> >> spotless at the actual Google Java Style template, and I think it makes
> >> both of the examples you found look better.
> >>
> >>> On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
> >>>
> >>> +1 for adding this to ./gradlew build
> >>>
> >>> But I think we might want to fix up the formatter a bit before
> >> reformatting
> >>> the code. I tried running spotlessApply, and it did some unfortunate
> >>> reformatting of code to make it less readable.
> >>>
> >>> One problem is with method chaining. We have a few different factory
> APIs
> >>> that encourage method chaining, and it put all the method calls on a
> >> single
> >>> line. For example here's one change:
> >>>
> >>> -ClientCacheFactory ccf = new ClientCacheFactory()
> >>> -
> >>> .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),
> port)
> >>> -.set(SECURITY_CLIENT_AUTH_INIT,
> >>> UserPasswordAuthInit.class.getName() + ".create")
> >>> -.set(SECURITY_PREFIX+"username", "root")
> >>> -.set(SECURITY_PREFIX+"password", "root");
> >>> +ClientCacheFactory ccf = new
> >>> ClientCacheFactory().addPoolServer(NetworkUtils.
> >> getServerHostName(server1.getHost()),
> >>> port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.
> getName()
> >> +
> >>> ".create").set(SECURITY_PREFIX + "username",
> "root").set(SECURITY_PREFIX
> >> +
> >>> "password", "root");
> >>>
> >>> I see a similar problem where it put array initialization all on a
> single
> >>> line:
> >>>
> >>> +  public void testMultiColOrderByWithIndexResultWithProjection()
> throws
> >>> Exception {
> >>>String queries[] = {
> >>>// Test case No. IUMR021
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 order by ID desc, pkid desc ",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 order by ID asc, pkid asc ",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 and ID < 20 order by ID asc, pkid asc ",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 and ID < 20 order by ID desc , pkid desc",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID != 10 order by ID asc , pkid desc",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID != 10 order by ID desc, pkid asc ",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 order by ID desc, pkid desc limit 5",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 order by ID asc, pkid asc limit 5",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID > 10 and ID < 20 order by ID desc, pkid asc limit 5",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID >= 10 and ID <= 20 order by ID desc, pkid desc limit 5",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID >= 10 and ID <= 20 order by ID asc, pkid asc limit 5",
> >>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1
> >>> where ID != 10 order by ID asc , pkid desc limit 

Re: Coding practices/standards

[Jared Stewart]

To give everyone an update, using the Google Java Style eclipse template there 
is an issue spotlessCheck where fails for 
geode-core/src/test/java/org/apache/geode/cache30/Bug38741DUnitTest.java even 
if you run it directly after spotlessApply. This needs to be investigated and 
fixed before I can open a pull request to enable spotless.

  
> On Oct 14, 2016, at 4:57 PM, Dan Smith  wrote:
> 
> +1 - The formatting looks better now.
> 
> -Dan
> 
> On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart  wrote:
> 
>> I agree that the formatter needs fixing up.  Our wiki <
>> https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide> says
>> that we follow the Google Java Style guide, but that is not actually what’s
>> in our formatter templates.  I pushed a new branch > jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that points
>> spotless at the actual Google Java Style template, and I think it makes
>> both of the examples you found look better.
>> 
>>> On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
>>> 
>>> +1 for adding this to ./gradlew build
>>> 
>>> But I think we might want to fix up the formatter a bit before
>> reformatting
>>> the code. I tried running spotlessApply, and it did some unfortunate
>>> reformatting of code to make it less readable.
>>> 
>>> One problem is with method chaining. We have a few different factory APIs
>>> that encourage method chaining, and it put all the method calls on a
>> single
>>> line. For example here's one change:
>>> 
>>> -ClientCacheFactory ccf = new ClientCacheFactory()
>>> -
>>> .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()), port)
>>> -.set(SECURITY_CLIENT_AUTH_INIT,
>>> UserPasswordAuthInit.class.getName() + ".create")
>>> -.set(SECURITY_PREFIX+"username", "root")
>>> -.set(SECURITY_PREFIX+"password", "root");
>>> +ClientCacheFactory ccf = new
>>> ClientCacheFactory().addPoolServer(NetworkUtils.
>> getServerHostName(server1.getHost()),
>>> port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.getName()
>> +
>>> ".create").set(SECURITY_PREFIX + "username", "root").set(SECURITY_PREFIX
>> +
>>> "password", "root");
>>> 
>>> I see a similar problem where it put array initialization all on a single
>>> line:
>>> 
>>> +  public void testMultiColOrderByWithIndexResultWithProjection() throws
>>> Exception {
>>>String queries[] = {
>>>// Test case No. IUMR021
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 order by ID desc, pkid desc ",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 order by ID asc, pkid asc ",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 and ID < 20 order by ID asc, pkid asc ",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 and ID < 20 order by ID desc , pkid desc",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID != 10 order by ID asc , pkid desc",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID != 10 order by ID desc, pkid asc ",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 order by ID desc, pkid desc limit 5",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 order by ID asc, pkid asc limit 5",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 and ID < 20 order by ID desc, pkid asc limit 5",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID >= 10 and ID <= 20 order by ID desc, pkid desc limit 5",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID >= 10 and ID <= 20 order by ID asc, pkid asc limit 5",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID != 10 order by ID asc , pkid desc limit 10",
>>> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID != 10 order by ID desc, pkid desc limit 10",
>>> -
>>> -   };
>>> +"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
>>> where ID > 10 order by ID desc, pkid desc ", "SELECT   ID, description,
>>> createTime, pkid FROM /portfolio1 pf1 where ID > 10 order by ID asc, pkid
>>> asc ", "SELECT   ID, 

Re: Coding practices/standards

[Dan Smith]

+1 - The formatting looks better now.

-Dan

On Thu, Oct 13, 2016 at 11:06 AM, Jared Stewart  wrote:

> I agree that the formatter needs fixing up.  Our wiki <
> https://cwiki.apache.org/confluence/display/GEODE/Code+Style+Guide> says
> that we follow the Google Java Style guide, but that is not actually what’s
> in our formatter templates.  I pushed a new branch  jaredjstewart/incubator-geode/tree/spotlessPluginGoogleStyle> that points
> spotless at the actual Google Java Style template, and I think it makes
> both of the examples you found look better.
>
> > On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
> >
> > +1 for adding this to ./gradlew build
> >
> > But I think we might want to fix up the formatter a bit before
> reformatting
> > the code. I tried running spotlessApply, and it did some unfortunate
> > reformatting of code to make it less readable.
> >
> > One problem is with method chaining. We have a few different factory APIs
> > that encourage method chaining, and it put all the method calls on a
> single
> > line. For example here's one change:
> >
> > -ClientCacheFactory ccf = new ClientCacheFactory()
> > -
> > .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()), port)
> > -.set(SECURITY_CLIENT_AUTH_INIT,
> > UserPasswordAuthInit.class.getName() + ".create")
> > -.set(SECURITY_PREFIX+"username", "root")
> > -.set(SECURITY_PREFIX+"password", "root");
> > +ClientCacheFactory ccf = new
> > ClientCacheFactory().addPoolServer(NetworkUtils.
> getServerHostName(server1.getHost()),
> > port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.getName()
> +
> > ".create").set(SECURITY_PREFIX + "username", "root").set(SECURITY_PREFIX
> +
> > "password", "root");
> >
> > I see a similar problem where it put array initialization all on a single
> > line:
> >
> > +  public void testMultiColOrderByWithIndexResultWithProjection() throws
> > Exception {
> > String queries[] = {
> > // Test case No. IUMR021
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 order by ID desc, pkid desc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 order by ID asc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 and ID < 20 order by ID asc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 and ID < 20 order by ID desc , pkid desc",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID != 10 order by ID asc , pkid desc",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID != 10 order by ID desc, pkid asc ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 order by ID desc, pkid desc limit 5",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 order by ID asc, pkid asc limit 5",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 and ID < 20 order by ID desc, pkid asc limit 5",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID >= 10 and ID <= 20 order by ID desc, pkid desc limit 5",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID >= 10 and ID <= 20 order by ID asc, pkid asc limit 5",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID != 10 order by ID asc , pkid desc limit 10",
> > -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID != 10 order by ID desc, pkid desc limit 10",
> > -
> > -   };
> > +"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 order by ID desc, pkid desc ", "SELECT   ID, description,
> > createTime, pkid FROM /portfolio1 pf1 where ID > 10 order by ID asc, pkid
> > asc ", "SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> > where ID > 10 and ID < 20 order by ID asc, pkid asc ", "SELECT   ID,
> > description, createTime, pkid FROM /portfolio1 pf1 where ID > 10 and ID <
> > 20 order by ID desc , pkid desc", "SELECT   ID, description, createTime,
> > pkid FROM /portfolio1 pf1 where ID >= 10 and ID <= 20 order by ID desc,
> > pkid asc ", "SELECT   ID, description, createTime, pkid FROM /portfolio1
> > pf1 where ID >= 

Re: Coding practices/standards

[Bruce Schuchardt]

+1
The Google style guide is what we chose for Geode when we couldn't use 
the Elements book


Le 10/14/2016 à 4:03 PM, Kirk Lund a écrit :

+1 to a) embracing https://google.github.io/styleguide/javaguide.html 100%,
b) adopting the google formatters for IntelliJ and Eclipse, c)using spotless


On Fri, Oct 14, 2016 at 6:56 AM, Jacob Barrett  wrote:


+1

On Thu, Oct 13, 2016 at 10:04 AM Kevin Duling  wrote:


Given that, +1 from me!

On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart 
wrote:


The task is fully suppressible with -x spotlessCheck.  Also, if you

have

any formatter errors you can automatically fix them with 'gradle
spotlessApply’.


On Oct 13, 2016, at 9:40 AM, Kevin Duling 

wrote:

If we made formatting a warning, then people would probably quickly

ignore

it.
If we made formatting an error, we need to be sure we don't get in to

the

situation where 's formatter is not in agreement

with

the

build's checker.

I can live with an additional 17 seconds as well.  And Jared's

already

reduced the build time locally by 50%.  But I still want the ability

to

suppress the check similar to -x javadoc.

On Wed, Oct 12, 2016 at 9:58 PM, William Markito <

wmark...@pivotal.io>

wrote:


This sounds really good to me as well.  +1

On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart 

wrote:

Agree with Mark, this has to work with 0 errors before it would be

useful in precheckin. I think I could live with an additional 17

seconds

most of the time for running the spotlessCheck as suggested.

Jared, Is that 17 seconds running locally on your laptop or on a

more

capable machine?

Ken


On Oct 12, 2016, at 3:39 PM, Jared Stewart 

wrote:

If you want to try it out, I pushed a branch to my Geode repo

that

contains this change:

https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin

<

https://github.com/jaredjstewart/incubator-geode/

tree/spotlessPlugin

On Oct 12, 2016, at 2:27 PM, Darrel Schneider <

dschnei...@pivotal.io

wrote:

I like Dan's idea of catching formatting issues earlier but I

think

adding

5-10 minutes to "build" would be too much. Currently when I'm

trying

to do

a quick build I use -xjavadoc. I'd probably do the same for this

target if

it was part of build until I'm ready to do a precheckin.

Mark, wouldn't running the formatter on all our java files and

checking

them in get these issues down to 0?

On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <

ukohlme...@pivotal.io

wrote:


+1 - adding checkstyle to precheckin.

If the developer uses the provided templates ( eclipse +

intellij)

then

most of the formatting issues should be handled before

precheckin.

Also, if

a developer has a questionable coding style, that should lessen

as

that

developer will have resolve the issues before being able to

commit.

I also believe that this should not be an overbearing and

intrusive

process.

--Udo



On 13/10/16 6:36 am, Mark Bretl wrote:


Dan,

There is some extra amount of time, 5-10 minutes extra for the

entire

project (depending on your CPU). I think the real issue to

adding

it

to

the
precheckin target and have it be 'effective' is it needs to

run

successfully, otherwise it would turn into noise most of the

time

I

think.

We need to get the issues down to 0 or manage to set a new

baseline

(not

the best idea), which is a lot of work, to make it run

successfully.

Right

now, if you run the target, it will fail every time since

there

outstanding
issues in the code and very hard to tell what issues were

introduced.


--Mark

On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith <

dsm...@pivotal.io>

wrote:

Seems like it should run as part of the build target. The only

reason to

make it part of precheckin is if it takes a long time,

otherwise

people

should get fast feedback they need to change their code

before

they

push.

-Dan

On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <

jstew...@pivotal.io>

wrote:

+1 to running during the precheckin target as well as Travis

CI

On Oct 12, 2016 11:20 AM, "Darrel Schneider" <

dschnei...@pivotal.io>

wrote:

If Travis CI is only run on pull requests then that is not

enough

because
committers do not submit pull requests. Having it run during

the

gradle

build or precheckin target is also needed. In addition to

that

I

also

wanted PRs to be checked.


On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart <

jstew...@pivotal.io>

wrote:

It would certainly be necessary to make sure that the code

style

to

be

enforced is sensible, e.g. doe not use wildcard imports.  We

would

also

want to 

Re: Coding practices/standards

[Kirk Lund]

+1 to a) embracing https://google.github.io/styleguide/javaguide.html 100%,
b) adopting the google formatters for IntelliJ and Eclipse, c)using spotless


On Fri, Oct 14, 2016 at 6:56 AM, Jacob Barrett  wrote:

> +1
>
> On Thu, Oct 13, 2016 at 10:04 AM Kevin Duling  wrote:
>
> > Given that, +1 from me!
> >
> > On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart 
> > wrote:
> >
> > > The task is fully suppressible with -x spotlessCheck.  Also, if you
> have
> > > any formatter errors you can automatically fix them with 'gradle
> > > spotlessApply’.
> > >
> > > > On Oct 13, 2016, at 9:40 AM, Kevin Duling 
> wrote:
> > > >
> > > > If we made formatting a warning, then people would probably quickly
> > > ignore
> > > > it.
> > > > If we made formatting an error, we need to be sure we don't get in to
> > the
> > > > situation where 's formatter is not in agreement
> with
> > > the
> > > > build's checker.
> > > >
> > > > I can live with an additional 17 seconds as well.  And Jared's
> already
> > > > reduced the build time locally by 50%.  But I still want the ability
> to
> > > > suppress the check similar to -x javadoc.
> > > >
> > > > On Wed, Oct 12, 2016 at 9:58 PM, William Markito <
> wmark...@pivotal.io>
> > > > wrote:
> > > >
> > > >> This sounds really good to me as well.  +1
> > > >>
> > > >> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart  >
> > > >> wrote:
> > > >>
> > > >>> This is running locally on my laptop.  Since Spotless is only doing
> > > code
> > > >>> formatting and not any other static analysis, it already has 0
> > errors.
> > > >>> (Other than, of course, formatting not consistent with the
> template.)
> > > >>>
> > >  On Oct 12, 2016, at 4:11 PM, Kenneth Howe 
> wrote:
> > > 
> > >  Agree with Mark, this has to work with 0 errors before it would be
> > > >>> useful in precheckin. I think I could live with an additional 17
> > > seconds
> > > >>> most of the time for running the spotlessCheck as suggested.
> > > 
> > >  Jared, Is that 17 seconds running locally on your laptop or on a
> > more
> > > >>> capable machine?
> > > 
> > >  Ken
> > > 
> > > > On Oct 12, 2016, at 3:39 PM, Jared Stewart 
> > > >> wrote:
> > > >
> > > > If you want to try it out, I pushed a branch to my Geode repo
> that
> > > >>> contains this change:
> > > >
> > https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
> > > >> <
> > > >>> https://github.com/jaredjstewart/incubator-geode/
> tree/spotlessPlugin
> > >
> > > >
> > > >> On Oct 12, 2016, at 2:27 PM, Darrel Schneider <
> > > dschnei...@pivotal.io
> > > >>>
> > > >>> wrote:
> > > >>
> > > >> I like Dan's idea of catching formatting issues earlier but I
> > think
> > > >>> adding
> > > >> 5-10 minutes to "build" would be too much. Currently when I'm
> > trying
> > > >>> to do
> > > >> a quick build I use -xjavadoc. I'd probably do the same for this
> > > >>> target if
> > > >> it was part of build until I'm ready to do a precheckin.
> > > >>
> > > >> Mark, wouldn't running the formatter on all our java files and
> > > >> checking
> > > >> them in get these issues down to 0?
> > > >>
> > > >> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <
> > > >> ukohlme...@pivotal.io
> > > 
> > > >> wrote:
> > > >>
> > > >>> +1 - adding checkstyle to precheckin.
> > > >>>
> > > >>> If the developer uses the provided templates ( eclipse +
> > intellij)
> > > >>> then
> > > >>> most of the formatting issues should be handled before
> > precheckin.
> > > >>> Also, if
> > > >>> a developer has a questionable coding style, that should lessen
> > as
> > > >>> that
> > > >>> developer will have resolve the issues before being able to
> > commit.
> > > >>>
> > > >>> I also believe that this should not be an overbearing and
> > intrusive
> > > >>> process.
> > > >>>
> > > >>> --Udo
> > > >>>
> > > >>>
> > > >>>
> > > >>> On 13/10/16 6:36 am, Mark Bretl wrote:
> > > >>>
> > >  Dan,
> > > 
> > >  There is some extra amount of time, 5-10 minutes extra for the
> > > >> entire
> > >  project (depending on your CPU). I think the real issue to
> > adding
> > > >> it
> > > >>> to
> > >  the
> > >  precheckin target and have it be 'effective' is it needs to
> run
> > >  successfully, otherwise it would turn into noise most of the
> > time
> > > I
> > > >>> think.
> > >  We need to get the issues down to 0 or manage to set a new
> > > baseline
> > > >>> (not
> > >  the best idea), which is a lot of work, to make it run
> > > >> successfully.
> > > >>> Right
> > >  now, if you run the target, it will fail every time since
> there
> > >  outstanding
> > >  issues in the code 

Re: Coding practices/standards

[Jacob Barrett]

+1

On Thu, Oct 13, 2016 at 10:04 AM Kevin Duling  wrote:

> Given that, +1 from me!
>
> On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart 
> wrote:
>
> > The task is fully suppressible with -x spotlessCheck.  Also, if you have
> > any formatter errors you can automatically fix them with 'gradle
> > spotlessApply’.
> >
> > > On Oct 13, 2016, at 9:40 AM, Kevin Duling  wrote:
> > >
> > > If we made formatting a warning, then people would probably quickly
> > ignore
> > > it.
> > > If we made formatting an error, we need to be sure we don't get in to
> the
> > > situation where 's formatter is not in agreement with
> > the
> > > build's checker.
> > >
> > > I can live with an additional 17 seconds as well.  And Jared's already
> > > reduced the build time locally by 50%.  But I still want the ability to
> > > suppress the check similar to -x javadoc.
> > >
> > > On Wed, Oct 12, 2016 at 9:58 PM, William Markito 
> > > wrote:
> > >
> > >> This sounds really good to me as well.  +1
> > >>
> > >> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart 
> > >> wrote:
> > >>
> > >>> This is running locally on my laptop.  Since Spotless is only doing
> > code
> > >>> formatting and not any other static analysis, it already has 0
> errors.
> > >>> (Other than, of course, formatting not consistent with the template.)
> > >>>
> >  On Oct 12, 2016, at 4:11 PM, Kenneth Howe  wrote:
> > 
> >  Agree with Mark, this has to work with 0 errors before it would be
> > >>> useful in precheckin. I think I could live with an additional 17
> > seconds
> > >>> most of the time for running the spotlessCheck as suggested.
> > 
> >  Jared, Is that 17 seconds running locally on your laptop or on a
> more
> > >>> capable machine?
> > 
> >  Ken
> > 
> > > On Oct 12, 2016, at 3:39 PM, Jared Stewart 
> > >> wrote:
> > >
> > > If you want to try it out, I pushed a branch to my Geode repo that
> > >>> contains this change:
> > >
> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
> > >> <
> > >>> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
> >
> > >
> > >> On Oct 12, 2016, at 2:27 PM, Darrel Schneider <
> > dschnei...@pivotal.io
> > >>>
> > >>> wrote:
> > >>
> > >> I like Dan's idea of catching formatting issues earlier but I
> think
> > >>> adding
> > >> 5-10 minutes to "build" would be too much. Currently when I'm
> trying
> > >>> to do
> > >> a quick build I use -xjavadoc. I'd probably do the same for this
> > >>> target if
> > >> it was part of build until I'm ready to do a precheckin.
> > >>
> > >> Mark, wouldn't running the formatter on all our java files and
> > >> checking
> > >> them in get these issues down to 0?
> > >>
> > >> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <
> > >> ukohlme...@pivotal.io
> > 
> > >> wrote:
> > >>
> > >>> +1 - adding checkstyle to precheckin.
> > >>>
> > >>> If the developer uses the provided templates ( eclipse +
> intellij)
> > >>> then
> > >>> most of the formatting issues should be handled before
> precheckin.
> > >>> Also, if
> > >>> a developer has a questionable coding style, that should lessen
> as
> > >>> that
> > >>> developer will have resolve the issues before being able to
> commit.
> > >>>
> > >>> I also believe that this should not be an overbearing and
> intrusive
> > >>> process.
> > >>>
> > >>> --Udo
> > >>>
> > >>>
> > >>>
> > >>> On 13/10/16 6:36 am, Mark Bretl wrote:
> > >>>
> >  Dan,
> > 
> >  There is some extra amount of time, 5-10 minutes extra for the
> > >> entire
> >  project (depending on your CPU). I think the real issue to
> adding
> > >> it
> > >>> to
> >  the
> >  precheckin target and have it be 'effective' is it needs to run
> >  successfully, otherwise it would turn into noise most of the
> time
> > I
> > >>> think.
> >  We need to get the issues down to 0 or manage to set a new
> > baseline
> > >>> (not
> >  the best idea), which is a lot of work, to make it run
> > >> successfully.
> > >>> Right
> >  now, if you run the target, it will fail every time since there
> >  outstanding
> >  issues in the code and very hard to tell what issues were
> > >> introduced.
> > 
> > 
> >  --Mark
> > 
> >  On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith 
> > >>> wrote:
> > 
> >  Seems like it should run as part of the build target. The only
> > >>> reason to
> > > make it part of precheckin is if it takes a long time,
> otherwise
> > >>> people
> > > should get fast feedback they need to change their code before
> > >> they
> > >>> push.
> > >
> > 

Re: Coding practices/standards

[Jared Stewart]

I agree that the formatter needs fixing up.  Our wiki 
 says that 
we follow the Google Java Style guide, but that is not actually what’s in our 
formatter templates.  I pushed a new branch 

 that points spotless at the actual Google Java Style template, and I think it 
makes both of the examples you found look better.

> On Oct 13, 2016, at 10:11 AM, Dan Smith  wrote:
> 
> +1 for adding this to ./gradlew build
> 
> But I think we might want to fix up the formatter a bit before reformatting
> the code. I tried running spotlessApply, and it did some unfortunate
> reformatting of code to make it less readable.
> 
> One problem is with method chaining. We have a few different factory APIs
> that encourage method chaining, and it put all the method calls on a single
> line. For example here's one change:
> 
> -ClientCacheFactory ccf = new ClientCacheFactory()
> -
> .addPoolServer(NetworkUtils.getServerHostName(server1.getHost()), port)
> -.set(SECURITY_CLIENT_AUTH_INIT,
> UserPasswordAuthInit.class.getName() + ".create")
> -.set(SECURITY_PREFIX+"username", "root")
> -.set(SECURITY_PREFIX+"password", "root");
> +ClientCacheFactory ccf = new
> ClientCacheFactory().addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),
> port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.getName() +
> ".create").set(SECURITY_PREFIX + "username", "root").set(SECURITY_PREFIX +
> "password", "root");
> 
> I see a similar problem where it put array initialization all on a single
> line:
> 
> +  public void testMultiColOrderByWithIndexResultWithProjection() throws
> Exception {
> String queries[] = {
> // Test case No. IUMR021
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 order by ID desc, pkid desc ",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 order by ID asc, pkid asc ",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 and ID < 20 order by ID asc, pkid asc ",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 and ID < 20 order by ID desc , pkid desc",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID != 10 order by ID asc , pkid desc",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID != 10 order by ID desc, pkid asc ",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 order by ID desc, pkid desc limit 5",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 order by ID asc, pkid asc limit 5",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 and ID < 20 order by ID desc, pkid asc limit 5",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID >= 10 and ID <= 20 order by ID desc, pkid desc limit 5",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID >= 10 and ID <= 20 order by ID asc, pkid asc limit 5",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID != 10 order by ID asc , pkid desc limit 10",
> -"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID != 10 order by ID desc, pkid desc limit 10",
> -
> -   };
> +"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 order by ID desc, pkid desc ", "SELECT   ID, description,
> createTime, pkid FROM /portfolio1 pf1 where ID > 10 order by ID asc, pkid
> asc ", "SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
> where ID > 10 and ID < 20 order by ID asc, pkid asc ", "SELECT   ID,
> description, createTime, pkid FROM /portfolio1 pf1 where ID > 10 and ID <
> 20 order by ID desc , pkid desc", "SELECT   ID, description, createTime,
> pkid FROM /portfolio1 pf1 where ID >= 10 and ID <= 20 order by ID desc,
> pkid asc ", "SELECT   ID, description, createTime, pkid FROM /portfolio1
> pf1 where ID >= 10 and ID <= 20 order by ID asc, pkid desc", "SELECT   ID,
> description, createTime, pkid FROM /portfolio1 pf1 where ID != 10 order by
> ID asc , pkid desc", "SELECT   ID, description, createTime, pkid FROM
> /portfolio1 pf1 where ID != 10 order by ID desc, pkid asc ",
> +"SELECT  

Re: Coding practices/standards

[Dan Smith]

+1 for adding this to ./gradlew build

But I think we might want to fix up the formatter a bit before reformatting
the code. I tried running spotlessApply, and it did some unfortunate
reformatting of code to make it less readable.

One problem is with method chaining. We have a few different factory APIs
that encourage method chaining, and it put all the method calls on a single
line. For example here's one change:

-ClientCacheFactory ccf = new ClientCacheFactory()
-
.addPoolServer(NetworkUtils.getServerHostName(server1.getHost()), port)
-.set(SECURITY_CLIENT_AUTH_INIT,
UserPasswordAuthInit.class.getName() + ".create")
-.set(SECURITY_PREFIX+"username", "root")
-.set(SECURITY_PREFIX+"password", "root");
+ClientCacheFactory ccf = new
ClientCacheFactory().addPoolServer(NetworkUtils.getServerHostName(server1.getHost()),
port).set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.getName() +
".create").set(SECURITY_PREFIX + "username", "root").set(SECURITY_PREFIX +
"password", "root");

I see a similar problem where it put array initialization all on a single
line:

+  public void testMultiColOrderByWithIndexResultWithProjection() throws
Exception {
 String queries[] = {
 // Test case No. IUMR021
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 order by ID desc, pkid desc ",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 and ID < 20 order by ID asc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 and ID < 20 order by ID desc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID >= 10 and ID <= 20 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID >= 10 and ID <= 20 order by ID asc, pkid desc",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID != 10 order by ID asc , pkid desc",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID != 10 order by ID desc, pkid asc ",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 order by ID desc, pkid desc limit 5",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 order by ID asc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 and ID < 20 order by ID asc, pkid desc limit 5 ",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 and ID < 20 order by ID desc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID >= 10 and ID <= 20 order by ID desc, pkid desc limit 5",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID >= 10 and ID <= 20 order by ID asc, pkid asc limit 5",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID != 10 order by ID asc , pkid desc limit 10",
-"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID != 10 order by ID desc, pkid desc limit 10",
-
-   };
+"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 order by ID desc, pkid desc ", "SELECT   ID, description,
createTime, pkid FROM /portfolio1 pf1 where ID > 10 order by ID asc, pkid
asc ", "SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 and ID < 20 order by ID asc, pkid asc ", "SELECT   ID,
description, createTime, pkid FROM /portfolio1 pf1 where ID > 10 and ID <
20 order by ID desc , pkid desc", "SELECT   ID, description, createTime,
pkid FROM /portfolio1 pf1 where ID >= 10 and ID <= 20 order by ID desc,
pkid asc ", "SELECT   ID, description, createTime, pkid FROM /portfolio1
pf1 where ID >= 10 and ID <= 20 order by ID asc, pkid desc", "SELECT   ID,
description, createTime, pkid FROM /portfolio1 pf1 where ID != 10 order by
ID asc , pkid desc", "SELECT   ID, description, createTime, pkid FROM
/portfolio1 pf1 where ID != 10 order by ID desc, pkid asc ",
+"SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1
where ID > 10 order by ID desc, pkid desc limit 5", "SELECT   ID,
description, createTime, pkid FROM /portfolio1 pf1 where ID > 10 order by
ID asc, pkid asc limit 5", "SELECT   ID, description, createTime, pkid FROM
/portfolio1 pf1 where ID > 10 and ID < 20 order by ID asc, pkid desc limit
5 ", "SELECT   ID, description, createTime, pkid FROM /portfolio1 pf1 where
ID > 10 and ID < 20 order by ID desc, pkid asc limit 5", "SELECT   ID,
description, createTime, pkid FROM /portfolio1 pf1 where ID >= 10 and ID <=
20 order by ID desc, pkid desc limit 5", "SELECT   ID, description,
createTime, pkid FROM /portfolio1 pf1 where ID >= 10 and ID 

Re: Coding practices/standards

[Kevin Duling]

Given that, +1 from me!

On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart  wrote:

> The task is fully suppressible with -x spotlessCheck.  Also, if you have
> any formatter errors you can automatically fix them with 'gradle
> spotlessApply’.
>
> > On Oct 13, 2016, at 9:40 AM, Kevin Duling  wrote:
> >
> > If we made formatting a warning, then people would probably quickly
> ignore
> > it.
> > If we made formatting an error, we need to be sure we don't get in to the
> > situation where 's formatter is not in agreement with
> the
> > build's checker.
> >
> > I can live with an additional 17 seconds as well.  And Jared's already
> > reduced the build time locally by 50%.  But I still want the ability to
> > suppress the check similar to -x javadoc.
> >
> > On Wed, Oct 12, 2016 at 9:58 PM, William Markito 
> > wrote:
> >
> >> This sounds really good to me as well.  +1
> >>
> >> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart 
> >> wrote:
> >>
> >>> This is running locally on my laptop.  Since Spotless is only doing
> code
> >>> formatting and not any other static analysis, it already has 0 errors.
> >>> (Other than, of course, formatting not consistent with the template.)
> >>>
>  On Oct 12, 2016, at 4:11 PM, Kenneth Howe  wrote:
> 
>  Agree with Mark, this has to work with 0 errors before it would be
> >>> useful in precheckin. I think I could live with an additional 17
> seconds
> >>> most of the time for running the spotlessCheck as suggested.
> 
>  Jared, Is that 17 seconds running locally on your laptop or on a more
> >>> capable machine?
> 
>  Ken
> 
> > On Oct 12, 2016, at 3:39 PM, Jared Stewart 
> >> wrote:
> >
> > If you want to try it out, I pushed a branch to my Geode repo that
> >>> contains this change:
> > https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
> >> <
> >>> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin>
> >
> >> On Oct 12, 2016, at 2:27 PM, Darrel Schneider <
> dschnei...@pivotal.io
> >>>
> >>> wrote:
> >>
> >> I like Dan's idea of catching formatting issues earlier but I think
> >>> adding
> >> 5-10 minutes to "build" would be too much. Currently when I'm trying
> >>> to do
> >> a quick build I use -xjavadoc. I'd probably do the same for this
> >>> target if
> >> it was part of build until I'm ready to do a precheckin.
> >>
> >> Mark, wouldn't running the formatter on all our java files and
> >> checking
> >> them in get these issues down to 0?
> >>
> >> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <
> >> ukohlme...@pivotal.io
> 
> >> wrote:
> >>
> >>> +1 - adding checkstyle to precheckin.
> >>>
> >>> If the developer uses the provided templates ( eclipse + intellij)
> >>> then
> >>> most of the formatting issues should be handled before precheckin.
> >>> Also, if
> >>> a developer has a questionable coding style, that should lessen as
> >>> that
> >>> developer will have resolve the issues before being able to commit.
> >>>
> >>> I also believe that this should not be an overbearing and intrusive
> >>> process.
> >>>
> >>> --Udo
> >>>
> >>>
> >>>
> >>> On 13/10/16 6:36 am, Mark Bretl wrote:
> >>>
>  Dan,
> 
>  There is some extra amount of time, 5-10 minutes extra for the
> >> entire
>  project (depending on your CPU). I think the real issue to adding
> >> it
> >>> to
>  the
>  precheckin target and have it be 'effective' is it needs to run
>  successfully, otherwise it would turn into noise most of the time
> I
> >>> think.
>  We need to get the issues down to 0 or manage to set a new
> baseline
> >>> (not
>  the best idea), which is a lot of work, to make it run
> >> successfully.
> >>> Right
>  now, if you run the target, it will fail every time since there
>  outstanding
>  issues in the code and very hard to tell what issues were
> >> introduced.
> 
> 
>  --Mark
> 
>  On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith 
> >>> wrote:
> 
>  Seems like it should run as part of the build target. The only
> >>> reason to
> > make it part of precheckin is if it takes a long time, otherwise
> >>> people
> > should get fast feedback they need to change their code before
> >> they
> >>> push.
> >
> > -Dan
> >
> > On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <
> >>> jstew...@pivotal.io>
> > wrote:
> >
> > +1 to running during the precheckin target as well as Travis CI
> >>
> >> On Oct 12, 2016 11:20 AM, "Darrel Schneider" <
> >>> dschnei...@pivotal.io>
> >> wrote:
> >>
> >> If Travis 

Re: Coding practices/standards

[Jared Stewart]

The task is fully suppressible with -x spotlessCheck.  Also, if you have any 
formatter errors you can automatically fix them with 'gradle spotlessApply’.  

> On Oct 13, 2016, at 9:40 AM, Kevin Duling  wrote:
> 
> If we made formatting a warning, then people would probably quickly ignore
> it.
> If we made formatting an error, we need to be sure we don't get in to the
> situation where 's formatter is not in agreement with the
> build's checker.
> 
> I can live with an additional 17 seconds as well.  And Jared's already
> reduced the build time locally by 50%.  But I still want the ability to
> suppress the check similar to -x javadoc.
> 
> On Wed, Oct 12, 2016 at 9:58 PM, William Markito 
> wrote:
> 
>> This sounds really good to me as well.  +1
>> 
>> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart 
>> wrote:
>> 
>>> This is running locally on my laptop.  Since Spotless is only doing code
>>> formatting and not any other static analysis, it already has 0 errors.
>>> (Other than, of course, formatting not consistent with the template.)
>>> 
 On Oct 12, 2016, at 4:11 PM, Kenneth Howe  wrote:
 
 Agree with Mark, this has to work with 0 errors before it would be
>>> useful in precheckin. I think I could live with an additional 17 seconds
>>> most of the time for running the spotlessCheck as suggested.
 
 Jared, Is that 17 seconds running locally on your laptop or on a more
>>> capable machine?
 
 Ken
 
> On Oct 12, 2016, at 3:39 PM, Jared Stewart 
>> wrote:
> 
> If you want to try it out, I pushed a branch to my Geode repo that
>>> contains this change:
> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
>> <
>>> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin>
> 
>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider >> 
>>> wrote:
>> 
>> I like Dan's idea of catching formatting issues earlier but I think
>>> adding
>> 5-10 minutes to "build" would be too much. Currently when I'm trying
>>> to do
>> a quick build I use -xjavadoc. I'd probably do the same for this
>>> target if
>> it was part of build until I'm ready to do a precheckin.
>> 
>> Mark, wouldn't running the formatter on all our java files and
>> checking
>> them in get these issues down to 0?
>> 
>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <
>> ukohlme...@pivotal.io
 
>> wrote:
>> 
>>> +1 - adding checkstyle to precheckin.
>>> 
>>> If the developer uses the provided templates ( eclipse + intellij)
>>> then
>>> most of the formatting issues should be handled before precheckin.
>>> Also, if
>>> a developer has a questionable coding style, that should lessen as
>>> that
>>> developer will have resolve the issues before being able to commit.
>>> 
>>> I also believe that this should not be an overbearing and intrusive
>>> process.
>>> 
>>> --Udo
>>> 
>>> 
>>> 
>>> On 13/10/16 6:36 am, Mark Bretl wrote:
>>> 
 Dan,
 
 There is some extra amount of time, 5-10 minutes extra for the
>> entire
 project (depending on your CPU). I think the real issue to adding
>> it
>>> to
 the
 precheckin target and have it be 'effective' is it needs to run
 successfully, otherwise it would turn into noise most of the time I
>>> think.
 We need to get the issues down to 0 or manage to set a new baseline
>>> (not
 the best idea), which is a lot of work, to make it run
>> successfully.
>>> Right
 now, if you run the target, it will fail every time since there
 outstanding
 issues in the code and very hard to tell what issues were
>> introduced.
 
 
 --Mark
 
 On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith 
>>> wrote:
 
 Seems like it should run as part of the build target. The only
>>> reason to
> make it part of precheckin is if it takes a long time, otherwise
>>> people
> should get fast feedback they need to change their code before
>> they
>>> push.
> 
> -Dan
> 
> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <
>>> jstew...@pivotal.io>
> wrote:
> 
> +1 to running during the precheckin target as well as Travis CI
>> 
>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" <
>>> dschnei...@pivotal.io>
>> wrote:
>> 
>> If Travis CI is only run on pull requests then that is not enough
>>> 
>> because
> 
>> committers do not submit pull requests. Having it run during the
>>> gradle
>>> build or precheckin target is also needed. In addition to that I
>>> also
>>> wanted PRs to be checked.
>>> 
>>> 
>>> On 

Re: Coding practices/standards

[Kevin Duling]

If we made formatting a warning, then people would probably quickly ignore
it.
If we made formatting an error, we need to be sure we don't get in to the
situation where 's formatter is not in agreement with the
build's checker.

I can live with an additional 17 seconds as well.  And Jared's already
reduced the build time locally by 50%.  But I still want the ability to
suppress the check similar to -x javadoc.

On Wed, Oct 12, 2016 at 9:58 PM, William Markito 
wrote:

> This sounds really good to me as well.  +1
>
> On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart 
> wrote:
>
> > This is running locally on my laptop.  Since Spotless is only doing code
> > formatting and not any other static analysis, it already has 0 errors.
> > (Other than, of course, formatting not consistent with the template.)
> >
> > > On Oct 12, 2016, at 4:11 PM, Kenneth Howe  wrote:
> > >
> > > Agree with Mark, this has to work with 0 errors before it would be
> > useful in precheckin. I think I could live with an additional 17 seconds
> > most of the time for running the spotlessCheck as suggested.
> > >
> > > Jared, Is that 17 seconds running locally on your laptop or on a more
> > capable machine?
> > >
> > > Ken
> > >
> > >> On Oct 12, 2016, at 3:39 PM, Jared Stewart 
> wrote:
> > >>
> > >> If you want to try it out, I pushed a branch to my Geode repo that
> > contains this change:
> > >> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin
> <
> > https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin>
> > >>
> > >>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider  >
> > wrote:
> > >>>
> > >>> I like Dan's idea of catching formatting issues earlier but I think
> > adding
> > >>> 5-10 minutes to "build" would be too much. Currently when I'm trying
> > to do
> > >>> a quick build I use -xjavadoc. I'd probably do the same for this
> > target if
> > >>> it was part of build until I'm ready to do a precheckin.
> > >>>
> > >>> Mark, wouldn't running the formatter on all our java files and
> checking
> > >>> them in get these issues down to 0?
> > >>>
> > >>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer <
> ukohlme...@pivotal.io
> > >
> > >>> wrote:
> > >>>
> >  +1 - adding checkstyle to precheckin.
> > 
> >  If the developer uses the provided templates ( eclipse + intellij)
> > then
> >  most of the formatting issues should be handled before precheckin.
> > Also, if
> >  a developer has a questionable coding style, that should lessen as
> > that
> >  developer will have resolve the issues before being able to commit.
> > 
> >  I also believe that this should not be an overbearing and intrusive
> >  process.
> > 
> >  --Udo
> > 
> > 
> > 
> >  On 13/10/16 6:36 am, Mark Bretl wrote:
> > 
> > > Dan,
> > >
> > > There is some extra amount of time, 5-10 minutes extra for the
> entire
> > > project (depending on your CPU). I think the real issue to adding
> it
> > to
> > > the
> > > precheckin target and have it be 'effective' is it needs to run
> > > successfully, otherwise it would turn into noise most of the time I
> > think.
> > > We need to get the issues down to 0 or manage to set a new baseline
> > (not
> > > the best idea), which is a lot of work, to make it run
> successfully.
> > Right
> > > now, if you run the target, it will fail every time since there
> > > outstanding
> > > issues in the code and very hard to tell what issues were
> introduced.
> > >
> > >
> > > --Mark
> > >
> > > On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith 
> > wrote:
> > >
> > > Seems like it should run as part of the build target. The only
> > reason to
> > >> make it part of precheckin is if it takes a long time, otherwise
> > people
> > >> should get fast feedback they need to change their code before
> they
> > push.
> > >>
> > >> -Dan
> > >>
> > >> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <
> > jstew...@pivotal.io>
> > >> wrote:
> > >>
> > >> +1 to running during the precheckin target as well as Travis CI
> > >>>
> > >>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" <
> > dschnei...@pivotal.io>
> > >>> wrote:
> > >>>
> > >>> If Travis CI is only run on pull requests then that is not enough
> > 
> > >>> because
> > >>
> > >>> committers do not submit pull requests. Having it run during the
> > gradle
> >  build or precheckin target is also needed. In addition to that I
> > also
> >  wanted PRs to be checked.
> > 
> > 
> >  On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart <
> > jstew...@pivotal.io>
> >  wrote:
> > 
> >  It would certainly be necessary to make sure that the code style
> > to
> > >
> >  be
> > 

Re: Coding practices/standards

[William Markito]

This sounds really good to me as well.  +1

On Wed, Oct 12, 2016 at 4:13 PM, Jared Stewart  wrote:

> This is running locally on my laptop.  Since Spotless is only doing code
> formatting and not any other static analysis, it already has 0 errors.
> (Other than, of course, formatting not consistent with the template.)
>
> > On Oct 12, 2016, at 4:11 PM, Kenneth Howe  wrote:
> >
> > Agree with Mark, this has to work with 0 errors before it would be
> useful in precheckin. I think I could live with an additional 17 seconds
> most of the time for running the spotlessCheck as suggested.
> >
> > Jared, Is that 17 seconds running locally on your laptop or on a more
> capable machine?
> >
> > Ken
> >
> >> On Oct 12, 2016, at 3:39 PM, Jared Stewart  wrote:
> >>
> >> If you want to try it out, I pushed a branch to my Geode repo that
> contains this change:
> >> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin <
> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin>
> >>
> >>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider 
> wrote:
> >>>
> >>> I like Dan's idea of catching formatting issues earlier but I think
> adding
> >>> 5-10 minutes to "build" would be too much. Currently when I'm trying
> to do
> >>> a quick build I use -xjavadoc. I'd probably do the same for this
> target if
> >>> it was part of build until I'm ready to do a precheckin.
> >>>
> >>> Mark, wouldn't running the formatter on all our java files and checking
> >>> them in get these issues down to 0?
> >>>
> >>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer  >
> >>> wrote:
> >>>
>  +1 - adding checkstyle to precheckin.
> 
>  If the developer uses the provided templates ( eclipse + intellij)
> then
>  most of the formatting issues should be handled before precheckin.
> Also, if
>  a developer has a questionable coding style, that should lessen as
> that
>  developer will have resolve the issues before being able to commit.
> 
>  I also believe that this should not be an overbearing and intrusive
>  process.
> 
>  --Udo
> 
> 
> 
>  On 13/10/16 6:36 am, Mark Bretl wrote:
> 
> > Dan,
> >
> > There is some extra amount of time, 5-10 minutes extra for the entire
> > project (depending on your CPU). I think the real issue to adding it
> to
> > the
> > precheckin target and have it be 'effective' is it needs to run
> > successfully, otherwise it would turn into noise most of the time I
> think.
> > We need to get the issues down to 0 or manage to set a new baseline
> (not
> > the best idea), which is a lot of work, to make it run successfully.
> Right
> > now, if you run the target, it will fail every time since there
> > outstanding
> > issues in the code and very hard to tell what issues were introduced.
> >
> >
> > --Mark
> >
> > On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith 
> wrote:
> >
> > Seems like it should run as part of the build target. The only
> reason to
> >> make it part of precheckin is if it takes a long time, otherwise
> people
> >> should get fast feedback they need to change their code before they
> push.
> >>
> >> -Dan
> >>
> >> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart <
> jstew...@pivotal.io>
> >> wrote:
> >>
> >> +1 to running during the precheckin target as well as Travis CI
> >>>
> >>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" <
> dschnei...@pivotal.io>
> >>> wrote:
> >>>
> >>> If Travis CI is only run on pull requests then that is not enough
> 
> >>> because
> >>
> >>> committers do not submit pull requests. Having it run during the
> gradle
>  build or precheckin target is also needed. In addition to that I
> also
>  wanted PRs to be checked.
> 
> 
>  On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart <
> jstew...@pivotal.io>
>  wrote:
> 
>  It would certainly be necessary to make sure that the code style
> to
> >
>  be
> >>
> >>> enforced is sensible, e.g. doe not use wildcard imports.  We would
> >
>  also
> >>
> >>> want to make one large commit to format all existing code before
> >
>  turning
> >>>
>  this on.
> >
> > Mark - Thank you for the information about the existing setup.
> >
> > Mark, Darrel, Kevin - Given all of your comments, I think it
> might
> >
>  make
> >>
> >>> more sense to add the flag to enable it in Travis CI rather than as
> >
>  part
> >>>
>  of  the build.  This way your build pass regardless of whitespace,
> >
>  but
> >>
> >>> the
> 
> > CI job would fail on PRs 

Re: Coding practices/standards

[Jared Stewart]

This is running locally on my laptop.  Since Spotless is only doing code 
formatting and not any other static analysis, it already has 0 errors.  (Other 
than, of course, formatting not consistent with the template.)

> On Oct 12, 2016, at 4:11 PM, Kenneth Howe  wrote:
> 
> Agree with Mark, this has to work with 0 errors before it would be useful in 
> precheckin. I think I could live with an additional 17 seconds most of the 
> time for running the spotlessCheck as suggested.
> 
> Jared, Is that 17 seconds running locally on your laptop or on a more capable 
> machine?
> 
> Ken
> 
>> On Oct 12, 2016, at 3:39 PM, Jared Stewart  wrote:
>> 
>> If you want to try it out, I pushed a branch to my Geode repo that contains 
>> this change:
>> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin 
>> 
>> 
>>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider  wrote:
>>> 
>>> I like Dan's idea of catching formatting issues earlier but I think adding
>>> 5-10 minutes to "build" would be too much. Currently when I'm trying to do
>>> a quick build I use -xjavadoc. I'd probably do the same for this target if
>>> it was part of build until I'm ready to do a precheckin.
>>> 
>>> Mark, wouldn't running the formatter on all our java files and checking
>>> them in get these issues down to 0?
>>> 
>>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer 
>>> wrote:
>>> 
 +1 - adding checkstyle to precheckin.
 
 If the developer uses the provided templates ( eclipse + intellij) then
 most of the formatting issues should be handled before precheckin. Also, if
 a developer has a questionable coding style, that should lessen as that
 developer will have resolve the issues before being able to commit.
 
 I also believe that this should not be an overbearing and intrusive
 process.
 
 --Udo
 
 
 
 On 13/10/16 6:36 am, Mark Bretl wrote:
 
> Dan,
> 
> There is some extra amount of time, 5-10 minutes extra for the entire
> project (depending on your CPU). I think the real issue to adding it to
> the
> precheckin target and have it be 'effective' is it needs to run
> successfully, otherwise it would turn into noise most of the time I think.
> We need to get the issues down to 0 or manage to set a new baseline (not
> the best idea), which is a lot of work, to make it run successfully. Right
> now, if you run the target, it will fail every time since there
> outstanding
> issues in the code and very hard to tell what issues were introduced.
> 
> 
> --Mark
> 
> On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith  wrote:
> 
> Seems like it should run as part of the build target. The only reason to
>> make it part of precheckin is if it takes a long time, otherwise people
>> should get fast feedback they need to change their code before they push.
>> 
>> -Dan
>> 
>> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart 
>> wrote:
>> 
>> +1 to running during the precheckin target as well as Travis CI
>>> 
>>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
>>> wrote:
>>> 
>>> If Travis CI is only run on pull requests then that is not enough
 
>>> because
>> 
>>> committers do not submit pull requests. Having it run during the gradle
 build or precheckin target is also needed. In addition to that I also
 wanted PRs to be checked.
 
 
 On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
 wrote:
 
 It would certainly be necessary to make sure that the code style to
> 
 be
>> 
>>> enforced is sensible, e.g. doe not use wildcard imports.  We would
> 
 also
>> 
>>> want to make one large commit to format all existing code before
> 
 turning
>>> 
 this on.
> 
> Mark - Thank you for the information about the existing setup.
> 
> Mark, Darrel, Kevin - Given all of your comments, I think it might
> 
 make
>> 
>>> more sense to add the flag to enable it in Travis CI rather than as
> 
 part
>>> 
 of  the build.  This way your build pass regardless of whitespace,
> 
 but
>> 
>>> the
 
> CI job would fail on PRs if they did not adhere to the standard
> 
 formatting.
 
> Anthony - It doesn’t seem to me that turning this on would have the
> 
 effect
 
> of combining reformatting commits and logic changes.  Rather, since
> 
 all
>> 
>>> code would already be formatted, there 

Re: Coding practices/standards

[Kenneth Howe]

Agree with Mark, this has to work with 0 errors before it would be useful in 
precheckin. I think I could live with an additional 17 seconds most of the time 
for running the spotlessCheck as suggested.

Jared, Is that 17 seconds running locally on your laptop or on a more capable 
machine?

Ken

> On Oct 12, 2016, at 3:39 PM, Jared Stewart  wrote:
> 
> If you want to try it out, I pushed a branch to my Geode repo that contains 
> this change:
> https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin 
> 
> 
>> On Oct 12, 2016, at 2:27 PM, Darrel Schneider  wrote:
>> 
>> I like Dan's idea of catching formatting issues earlier but I think adding
>> 5-10 minutes to "build" would be too much. Currently when I'm trying to do
>> a quick build I use -xjavadoc. I'd probably do the same for this target if
>> it was part of build until I'm ready to do a precheckin.
>> 
>> Mark, wouldn't running the formatter on all our java files and checking
>> them in get these issues down to 0?
>> 
>> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer 
>> wrote:
>> 
>>> +1 - adding checkstyle to precheckin.
>>> 
>>> If the developer uses the provided templates ( eclipse + intellij) then
>>> most of the formatting issues should be handled before precheckin. Also, if
>>> a developer has a questionable coding style, that should lessen as that
>>> developer will have resolve the issues before being able to commit.
>>> 
>>> I also believe that this should not be an overbearing and intrusive
>>> process.
>>> 
>>> --Udo
>>> 
>>> 
>>> 
>>> On 13/10/16 6:36 am, Mark Bretl wrote:
>>> 
 Dan,
 
 There is some extra amount of time, 5-10 minutes extra for the entire
 project (depending on your CPU). I think the real issue to adding it to
 the
 precheckin target and have it be 'effective' is it needs to run
 successfully, otherwise it would turn into noise most of the time I think.
 We need to get the issues down to 0 or manage to set a new baseline (not
 the best idea), which is a lot of work, to make it run successfully. Right
 now, if you run the target, it will fail every time since there
 outstanding
 issues in the code and very hard to tell what issues were introduced.
 
 
 --Mark
 
 On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith  wrote:
 
 Seems like it should run as part of the build target. The only reason to
> make it part of precheckin is if it takes a long time, otherwise people
> should get fast feedback they need to change their code before they push.
> 
> -Dan
> 
> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart 
> wrote:
> 
> +1 to running during the precheckin target as well as Travis CI
>> 
>> On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
>> wrote:
>> 
>> If Travis CI is only run on pull requests then that is not enough
>>> 
>> because
> 
>> committers do not submit pull requests. Having it run during the gradle
>>> build or precheckin target is also needed. In addition to that I also
>>> wanted PRs to be checked.
>>> 
>>> 
>>> On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
>>> wrote:
>>> 
>>> It would certainly be necessary to make sure that the code style to
 
>>> be
> 
>> enforced is sensible, e.g. doe not use wildcard imports.  We would
 
>>> also
> 
>> want to make one large commit to format all existing code before
 
>>> turning
>> 
>>> this on.
 
 Mark - Thank you for the information about the existing setup.
 
 Mark, Darrel, Kevin - Given all of your comments, I think it might
 
>>> make
> 
>> more sense to add the flag to enable it in Travis CI rather than as
 
>>> part
>> 
>>> of  the build.  This way your build pass regardless of whitespace,
 
>>> but
> 
>> the
>>> 
 CI job would fail on PRs if they did not adhere to the standard
 
>>> formatting.
>>> 
 Anthony - It doesn’t seem to me that turning this on would have the
 
>>> effect
>>> 
 of combining reformatting commits and logic changes.  Rather, since
 
>>> all
> 
>> code would already be formatted, there would no longer be any
 
>>> reformatting
>>> 
 commits except for single large commits when the code style file was
 updated.
 
 On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
> 
 bschucha...@pivotal.io
>> 
>>> wrote:
 
> I like the idea of doing this but I don't think Checkstyle should
> 
 be
> 
>> enabled until all 

Re: Coding practices/standards

[Jared Stewart]

If you want to try it out, I pushed a branch to my Geode repo that contains 
this change:
https://github.com/jaredjstewart/incubator-geode/tree/spotlessPlugin 


> On Oct 12, 2016, at 2:27 PM, Darrel Schneider  wrote:
> 
> I like Dan's idea of catching formatting issues earlier but I think adding
> 5-10 minutes to "build" would be too much. Currently when I'm trying to do
> a quick build I use -xjavadoc. I'd probably do the same for this target if
> it was part of build until I'm ready to do a precheckin.
> 
> Mark, wouldn't running the formatter on all our java files and checking
> them in get these issues down to 0?
> 
> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer 
> wrote:
> 
>> +1 - adding checkstyle to precheckin.
>> 
>> If the developer uses the provided templates ( eclipse + intellij) then
>> most of the formatting issues should be handled before precheckin. Also, if
>> a developer has a questionable coding style, that should lessen as that
>> developer will have resolve the issues before being able to commit.
>> 
>> I also believe that this should not be an overbearing and intrusive
>> process.
>> 
>> --Udo
>> 
>> 
>> 
>> On 13/10/16 6:36 am, Mark Bretl wrote:
>> 
>>> Dan,
>>> 
>>> There is some extra amount of time, 5-10 minutes extra for the entire
>>> project (depending on your CPU). I think the real issue to adding it to
>>> the
>>> precheckin target and have it be 'effective' is it needs to run
>>> successfully, otherwise it would turn into noise most of the time I think.
>>> We need to get the issues down to 0 or manage to set a new baseline (not
>>> the best idea), which is a lot of work, to make it run successfully. Right
>>> now, if you run the target, it will fail every time since there
>>> outstanding
>>> issues in the code and very hard to tell what issues were introduced.
>>> 
>>> 
>>> --Mark
>>> 
>>> On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith  wrote:
>>> 
>>> Seems like it should run as part of the build target. The only reason to
 make it part of precheckin is if it takes a long time, otherwise people
 should get fast feedback they need to change their code before they push.
 
 -Dan
 
 On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart 
 wrote:
 
 +1 to running during the precheckin target as well as Travis CI
> 
> On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
> wrote:
> 
> If Travis CI is only run on pull requests then that is not enough
>> 
> because
 
> committers do not submit pull requests. Having it run during the gradle
>> build or precheckin target is also needed. In addition to that I also
>> wanted PRs to be checked.
>> 
>> 
>> On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
>> wrote:
>> 
>> It would certainly be necessary to make sure that the code style to
>>> 
>> be
 
> enforced is sensible, e.g. doe not use wildcard imports.  We would
>>> 
>> also
 
> want to make one large commit to format all existing code before
>>> 
>> turning
> 
>> this on.
>>> 
>>> Mark - Thank you for the information about the existing setup.
>>> 
>>> Mark, Darrel, Kevin - Given all of your comments, I think it might
>>> 
>> make
 
> more sense to add the flag to enable it in Travis CI rather than as
>>> 
>> part
> 
>> of  the build.  This way your build pass regardless of whitespace,
>>> 
>> but
 
> the
>> 
>>> CI job would fail on PRs if they did not adhere to the standard
>>> 
>> formatting.
>> 
>>> Anthony - It doesn’t seem to me that turning this on would have the
>>> 
>> effect
>> 
>>> of combining reformatting commits and logic changes.  Rather, since
>>> 
>> all
 
> code would already be formatted, there would no longer be any
>>> 
>> reformatting
>> 
>>> commits except for single large commits when the code style file was
>>> updated.
>>> 
>>> On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
 
>>> bschucha...@pivotal.io
> 
>> wrote:
>>> 
 I like the idea of doing this but I don't think Checkstyle should
 
>>> be
 
> enabled until all of the code is reformatted.
>>> 
 Also, last time I checked there was still a problem with the
 
>>> IntelliJ
 
> auto-format settings.  It still used wildcard imports, which I
>>> 
>> believe
 
> we
> 
>> don't allow.  I've manually changed my settings in Editor->Code
>>> 
>> Style->Java
>> 
>>> to "Use single class import" to correct that problem.  I couldn't see
>>> 
>> how
> 
>> to get Gradle to do it.
>>> 
 

Re: Coding practices/standards

[Jared Stewart]

I did some investigation this afternoon on using the Gradle Spotless plugin 
rather than Checkstyle.  Whereas Checkstyle has its own formatting template 
syntax, Spotless can import our existing Eclipse formatter template.  The 
plugin adds tasks for “spotlessCheck” (which tells you whether or not your code 
is compliant) and “spotlessApply” (to automatically fix formatting errors for 
you).  By default, “spotlessCheck" runs as part of “build”. Spotless also runs 
much faster than Checkstyle - it only added roughly 17 seconds to my build.


> On Oct 12, 2016, at 2:27 PM, Darrel Schneider  wrote:
> 
> I like Dan's idea of catching formatting issues earlier but I think adding
> 5-10 minutes to "build" would be too much. Currently when I'm trying to do
> a quick build I use -xjavadoc. I'd probably do the same for this target if
> it was part of build until I'm ready to do a precheckin.
> 
> Mark, wouldn't running the formatter on all our java files and checking
> them in get these issues down to 0?
> 
> On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer 
> wrote:
> 
>> +1 - adding checkstyle to precheckin.
>> 
>> If the developer uses the provided templates ( eclipse + intellij) then
>> most of the formatting issues should be handled before precheckin. Also, if
>> a developer has a questionable coding style, that should lessen as that
>> developer will have resolve the issues before being able to commit.
>> 
>> I also believe that this should not be an overbearing and intrusive
>> process.
>> 
>> --Udo
>> 
>> 
>> 
>> On 13/10/16 6:36 am, Mark Bretl wrote:
>> 
>>> Dan,
>>> 
>>> There is some extra amount of time, 5-10 minutes extra for the entire
>>> project (depending on your CPU). I think the real issue to adding it to
>>> the
>>> precheckin target and have it be 'effective' is it needs to run
>>> successfully, otherwise it would turn into noise most of the time I think.
>>> We need to get the issues down to 0 or manage to set a new baseline (not
>>> the best idea), which is a lot of work, to make it run successfully. Right
>>> now, if you run the target, it will fail every time since there
>>> outstanding
>>> issues in the code and very hard to tell what issues were introduced.
>>> 
>>> 
>>> --Mark
>>> 
>>> On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith  wrote:
>>> 
>>> Seems like it should run as part of the build target. The only reason to
 make it part of precheckin is if it takes a long time, otherwise people
 should get fast feedback they need to change their code before they push.
 
 -Dan
 
 On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart 
 wrote:
 
 +1 to running during the precheckin target as well as Travis CI
> 
> On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
> wrote:
> 
> If Travis CI is only run on pull requests then that is not enough
>> 
> because
 
> committers do not submit pull requests. Having it run during the gradle
>> build or precheckin target is also needed. In addition to that I also
>> wanted PRs to be checked.
>> 
>> 
>> On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
>> wrote:
>> 
>> It would certainly be necessary to make sure that the code style to
>>> 
>> be
 
> enforced is sensible, e.g. doe not use wildcard imports.  We would
>>> 
>> also
 
> want to make one large commit to format all existing code before
>>> 
>> turning
> 
>> this on.
>>> 
>>> Mark - Thank you for the information about the existing setup.
>>> 
>>> Mark, Darrel, Kevin - Given all of your comments, I think it might
>>> 
>> make
 
> more sense to add the flag to enable it in Travis CI rather than as
>>> 
>> part
> 
>> of  the build.  This way your build pass regardless of whitespace,
>>> 
>> but
 
> the
>> 
>>> CI job would fail on PRs if they did not adhere to the standard
>>> 
>> formatting.
>> 
>>> Anthony - It doesn’t seem to me that turning this on would have the
>>> 
>> effect
>> 
>>> of combining reformatting commits and logic changes.  Rather, since
>>> 
>> all
 
> code would already be formatted, there would no longer be any
>>> 
>> reformatting
>> 
>>> commits except for single large commits when the code style file was
>>> updated.
>>> 
>>> On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
 
>>> bschucha...@pivotal.io
> 
>> wrote:
>>> 
 I like the idea of doing this but I don't think Checkstyle should
 
>>> be
 
> enabled until all of the code is reformatted.
>>> 
 Also, last time I checked there was still a problem with the
 
>>> IntelliJ
 
> auto-format settings.  It still used wildcard imports, 

Re: Coding practices/standards

[Darrel Schneider]

I like Dan's idea of catching formatting issues earlier but I think adding
5-10 minutes to "build" would be too much. Currently when I'm trying to do
a quick build I use -xjavadoc. I'd probably do the same for this target if
it was part of build until I'm ready to do a precheckin.

Mark, wouldn't running the formatter on all our java files and checking
them in get these issues down to 0?

On Wed, Oct 12, 2016 at 12:53 PM, Udo Kohlmeyer 
wrote:

> +1 - adding checkstyle to precheckin.
>
> If the developer uses the provided templates ( eclipse + intellij) then
> most of the formatting issues should be handled before precheckin. Also, if
> a developer has a questionable coding style, that should lessen as that
> developer will have resolve the issues before being able to commit.
>
> I also believe that this should not be an overbearing and intrusive
> process.
>
> --Udo
>
>
>
> On 13/10/16 6:36 am, Mark Bretl wrote:
>
>> Dan,
>>
>> There is some extra amount of time, 5-10 minutes extra for the entire
>> project (depending on your CPU). I think the real issue to adding it to
>> the
>> precheckin target and have it be 'effective' is it needs to run
>> successfully, otherwise it would turn into noise most of the time I think.
>> We need to get the issues down to 0 or manage to set a new baseline (not
>> the best idea), which is a lot of work, to make it run successfully. Right
>> now, if you run the target, it will fail every time since there
>> outstanding
>> issues in the code and very hard to tell what issues were introduced.
>>
>>
>> --Mark
>>
>> On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith  wrote:
>>
>> Seems like it should run as part of the build target. The only reason to
>>> make it part of precheckin is if it takes a long time, otherwise people
>>> should get fast feedback they need to change their code before they push.
>>>
>>> -Dan
>>>
>>> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart 
>>> wrote:
>>>
>>> +1 to running during the precheckin target as well as Travis CI

 On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
 wrote:

 If Travis CI is only run on pull requests then that is not enough
>
 because
>>>
 committers do not submit pull requests. Having it run during the gradle
> build or precheckin target is also needed. In addition to that I also
> wanted PRs to be checked.
>
>
> On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
> wrote:
>
> It would certainly be necessary to make sure that the code style to
>>
> be
>>>
 enforced is sensible, e.g. doe not use wildcard imports.  We would
>>
> also
>>>
 want to make one large commit to format all existing code before
>>
> turning

> this on.
>>
>> Mark - Thank you for the information about the existing setup.
>>
>> Mark, Darrel, Kevin - Given all of your comments, I think it might
>>
> make
>>>
 more sense to add the flag to enable it in Travis CI rather than as
>>
> part

> of  the build.  This way your build pass regardless of whitespace,
>>
> but
>>>
 the
>
>> CI job would fail on PRs if they did not adhere to the standard
>>
> formatting.
>
>> Anthony - It doesn’t seem to me that turning this on would have the
>>
> effect
>
>> of combining reformatting commits and logic changes.  Rather, since
>>
> all
>>>
 code would already be formatted, there would no longer be any
>>
> reformatting
>
>> commits except for single large commits when the code style file was
>> updated.
>>
>> On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
>>>
>> bschucha...@pivotal.io

> wrote:
>>
>>> I like the idea of doing this but I don't think Checkstyle should
>>>
>> be
>>>
 enabled until all of the code is reformatted.
>>
>>> Also, last time I checked there was still a problem with the
>>>
>> IntelliJ
>>>
 auto-format settings.  It still used wildcard imports, which I
>>
> believe
>>>
 we

> don't allow.  I've manually changed my settings in Editor->Code
>>
> Style->Java
>
>> to "Use single class import" to correct that problem.  I couldn't see
>>
> how

> to get Gradle to do it.
>>
>>>
>>> Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :
>>>
 Source code with a consistent look-and-feel makes it easier for

>>> people

> to join the project community and contribute.
>>
>>> Let’s continue to keep reformatting commits separate from logic

>>> changes—otherwise it’s too hard to review.
>>
>>> Anthony

 On Oct 12, 2016, at 10:06 AM, Dan Smith 
>
 wrote:
>>>
 +1
>
> This might be a good 

Re: Coding practices/standards

[Udo Kohlmeyer]

+1 - adding checkstyle to precheckin.

If the developer uses the provided templates ( eclipse + intellij) then 
most of the formatting issues should be handled before precheckin. Also, 
if a developer has a questionable coding style, that should lessen as 
that developer will have resolve the issues before being able to commit.


I also believe that this should not be an overbearing and intrusive 
process.


--Udo


On 13/10/16 6:36 am, Mark Bretl wrote:

Dan,

There is some extra amount of time, 5-10 minutes extra for the entire
project (depending on your CPU). I think the real issue to adding it to the
precheckin target and have it be 'effective' is it needs to run
successfully, otherwise it would turn into noise most of the time I think.
We need to get the issues down to 0 or manage to set a new baseline (not
the best idea), which is a lot of work, to make it run successfully. Right
now, if you run the target, it will fail every time since there outstanding
issues in the code and very hard to tell what issues were introduced.


--Mark

On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith  wrote:


Seems like it should run as part of the build target. The only reason to
make it part of precheckin is if it takes a long time, otherwise people
should get fast feedback they need to change their code before they push.

-Dan

On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart 
wrote:


+1 to running during the precheckin target as well as Travis CI

On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
wrote:


If Travis CI is only run on pull requests then that is not enough

because

committers do not submit pull requests. Having it run during the gradle
build or precheckin target is also needed. In addition to that I also
wanted PRs to be checked.


On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
wrote:


It would certainly be necessary to make sure that the code style to

be

enforced is sensible, e.g. doe not use wildcard imports.  We would

also

want to make one large commit to format all existing code before

turning

this on.

Mark - Thank you for the information about the existing setup.

Mark, Darrel, Kevin - Given all of your comments, I think it might

make

more sense to add the flag to enable it in Travis CI rather than as

part

of  the build.  This way your build pass regardless of whitespace,

but

the

CI job would fail on PRs if they did not adhere to the standard

formatting.

Anthony - It doesn’t seem to me that turning this on would have the

effect

of combining reformatting commits and logic changes.  Rather, since

all

code would already be formatted, there would no longer be any

reformatting

commits except for single large commits when the code style file was
updated.


On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <

bschucha...@pivotal.io

wrote:

I like the idea of doing this but I don't think Checkstyle should

be

enabled until all of the code is reformatted.

Also, last time I checked there was still a problem with the

IntelliJ

auto-format settings.  It still used wildcard imports, which I

believe

we

don't allow.  I've manually changed my settings in Editor->Code

Style->Java

to "Use single class import" to correct that problem.  I couldn't see

how

to get Gradle to do it.


Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :

Source code with a consistent look-and-feel makes it easier for

people

to join the project community and contribute.

Let’s continue to keep reformatting commits separate from logic

changes—otherwise it’s too hard to review.

Anthony


On Oct 12, 2016, at 10:06 AM, Dan Smith 

wrote:

+1

This might be a good time to reformat the code since I don't

think

there

are too many long lived feature branches outstanding.

-Dan

On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart <

jstew...@pivotal.io

wrote:

I would like to advocate for adding a Checkstyle <

http://checkstyle

.

sourceforge.net/> or Spotless  (and in the

intellij/eclipse

formatter xml files in our repository). This will alleviate

difficulties

reviewing code when whitespace or formatting has changed since

all

code

checked in will already comply with standards.

Re: Coding practices/standards

[Mark Bretl]

Dan,

There is some extra amount of time, 5-10 minutes extra for the entire
project (depending on your CPU). I think the real issue to adding it to the
precheckin target and have it be 'effective' is it needs to run
successfully, otherwise it would turn into noise most of the time I think.
We need to get the issues down to 0 or manage to set a new baseline (not
the best idea), which is a lot of work, to make it run successfully. Right
now, if you run the target, it will fail every time since there outstanding
issues in the code and very hard to tell what issues were introduced.


--Mark

On Wed, Oct 12, 2016 at 11:34 AM, Dan Smith  wrote:

> Seems like it should run as part of the build target. The only reason to
> make it part of precheckin is if it takes a long time, otherwise people
> should get fast feedback they need to change their code before they push.
>
> -Dan
>
> On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart 
> wrote:
>
> > +1 to running during the precheckin target as well as Travis CI
> >
> > On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
> > wrote:
> >
> > > If Travis CI is only run on pull requests then that is not enough
> because
> > > committers do not submit pull requests. Having it run during the gradle
> > > build or precheckin target is also needed. In addition to that I also
> > > wanted PRs to be checked.
> > >
> > >
> > > On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
> > > wrote:
> > >
> > > > It would certainly be necessary to make sure that the code style to
> be
> > > > enforced is sensible, e.g. doe not use wildcard imports.  We would
> also
> > > > want to make one large commit to format all existing code before
> > turning
> > > > this on.
> > > >
> > > > Mark - Thank you for the information about the existing setup.
> > > >
> > > > Mark, Darrel, Kevin - Given all of your comments, I think it might
> make
> > > > more sense to add the flag to enable it in Travis CI rather than as
> > part
> > > > of  the build.  This way your build pass regardless of whitespace,
> but
> > > the
> > > > CI job would fail on PRs if they did not adhere to the standard
> > > formatting.
> > > >
> > > > Anthony - It doesn’t seem to me that turning this on would have the
> > > effect
> > > > of combining reformatting commits and logic changes.  Rather, since
> all
> > > > code would already be formatted, there would no longer be any
> > > reformatting
> > > > commits except for single large commits when the code style file was
> > > > updated.
> > > >
> > > > > On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
> > bschucha...@pivotal.io
> > > >
> > > > wrote:
> > > > >
> > > > > I like the idea of doing this but I don't think Checkstyle should
> be
> > > > enabled until all of the code is reformatted.
> > > > >
> > > > > Also, last time I checked there was still a problem with the
> IntelliJ
> > > > auto-format settings.  It still used wildcard imports, which I
> believe
> > we
> > > > don't allow.  I've manually changed my settings in Editor->Code
> > > Style->Java
> > > > to "Use single class import" to correct that problem.  I couldn't see
> > how
> > > > to get Gradle to do it.
> > > > >
> > > > >
> > > > > Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :
> > > > >> Source code with a consistent look-and-feel makes it easier for
> > people
> > > > to join the project community and contribute.
> > > > >>
> > > > >> Let’s continue to keep reformatting commits separate from logic
> > > > changes—otherwise it’s too hard to review.
> > > > >>
> > > > >> Anthony
> > > > >>
> > > > >>> On Oct 12, 2016, at 10:06 AM, Dan Smith 
> wrote:
> > > > >>>
> > > > >>> +1
> > > > >>>
> > > > >>> This might be a good time to reformat the code since I don't
> think
> > > > there
> > > > >>> are too many long lived feature branches outstanding.
> > > > >>>
> > > > >>> -Dan
> > > > >>>
> > > > >>> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart <
> > jstew...@pivotal.io
> > > >
> > > > wrote:
> > > > >>>
> > > >  I would like to advocate for adding a Checkstyle <
> > http://checkstyle
> > > .
> > > >  sourceforge.net/> or Spotless  > spotless
> > > >
> > > >  gradle task to our build process to ensure that all code checked
> > in
> > > > meets
> > > >  the formatting standards described on the wiki <
> > > > https://cwiki.apache.org/
> > > >  confluence/display/GEODE/Code+Style+Guide> (and in the
> > > > intellij/eclipse
> > > >  formatter xml files in our repository). This will alleviate
> > > > difficulties
> > > >  reviewing code when whitespace or formatting has changed since
> all
> > > > code
> > > >  checked in will already comply with standards.
> > > > >
> > > >
> > > >
> > >
> >
>

Re: Coding practices/standards

[Dan Smith]

Seems like it should run as part of the build target. The only reason to
make it part of precheckin is if it takes a long time, otherwise people
should get fast feedback they need to change their code before they push.

-Dan

On Wed, Oct 12, 2016 at 11:24 AM, Jared Stewart  wrote:

> +1 to running during the precheckin target as well as Travis CI
>
> On Oct 12, 2016 11:20 AM, "Darrel Schneider" 
> wrote:
>
> > If Travis CI is only run on pull requests then that is not enough because
> > committers do not submit pull requests. Having it run during the gradle
> > build or precheckin target is also needed. In addition to that I also
> > wanted PRs to be checked.
> >
> >
> > On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
> > wrote:
> >
> > > It would certainly be necessary to make sure that the code style to be
> > > enforced is sensible, e.g. doe not use wildcard imports.  We would also
> > > want to make one large commit to format all existing code before
> turning
> > > this on.
> > >
> > > Mark - Thank you for the information about the existing setup.
> > >
> > > Mark, Darrel, Kevin - Given all of your comments, I think it might make
> > > more sense to add the flag to enable it in Travis CI rather than as
> part
> > > of  the build.  This way your build pass regardless of whitespace, but
> > the
> > > CI job would fail on PRs if they did not adhere to the standard
> > formatting.
> > >
> > > Anthony - It doesn’t seem to me that turning this on would have the
> > effect
> > > of combining reformatting commits and logic changes.  Rather, since all
> > > code would already be formatted, there would no longer be any
> > reformatting
> > > commits except for single large commits when the code style file was
> > > updated.
> > >
> > > > On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt <
> bschucha...@pivotal.io
> > >
> > > wrote:
> > > >
> > > > I like the idea of doing this but I don't think Checkstyle should be
> > > enabled until all of the code is reformatted.
> > > >
> > > > Also, last time I checked there was still a problem with the IntelliJ
> > > auto-format settings.  It still used wildcard imports, which I believe
> we
> > > don't allow.  I've manually changed my settings in Editor->Code
> > Style->Java
> > > to "Use single class import" to correct that problem.  I couldn't see
> how
> > > to get Gradle to do it.
> > > >
> > > >
> > > > Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :
> > > >> Source code with a consistent look-and-feel makes it easier for
> people
> > > to join the project community and contribute.
> > > >>
> > > >> Let’s continue to keep reformatting commits separate from logic
> > > changes—otherwise it’s too hard to review.
> > > >>
> > > >> Anthony
> > > >>
> > > >>> On Oct 12, 2016, at 10:06 AM, Dan Smith  wrote:
> > > >>>
> > > >>> +1
> > > >>>
> > > >>> This might be a good time to reformat the code since I don't think
> > > there
> > > >>> are too many long lived feature branches outstanding.
> > > >>>
> > > >>> -Dan
> > > >>>
> > > >>> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart <
> jstew...@pivotal.io
> > >
> > > wrote:
> > > >>>
> > >  I would like to advocate for adding a Checkstyle <
> http://checkstyle
> > .
> > >  sourceforge.net/> or Spotless  spotless
> > >
> > >  gradle task to our build process to ensure that all code checked
> in
> > > meets
> > >  the formatting standards described on the wiki <
> > > https://cwiki.apache.org/
> > >  confluence/display/GEODE/Code+Style+Guide> (and in the
> > > intellij/eclipse
> > >  formatter xml files in our repository). This will alleviate
> > > difficulties
> > >  reviewing code when whitespace or formatting has changed since all
> > > code
> > >  checked in will already comply with standards.
> > > >
> > >
> > >
> >
>

Re: Coding practices/standards

[Jared Stewart]

+1 to running during the precheckin target as well as Travis CI

On Oct 12, 2016 11:20 AM, "Darrel Schneider"  wrote:

> If Travis CI is only run on pull requests then that is not enough because
> committers do not submit pull requests. Having it run during the gradle
> build or precheckin target is also needed. In addition to that I also
> wanted PRs to be checked.
>
>
> On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart 
> wrote:
>
> > It would certainly be necessary to make sure that the code style to be
> > enforced is sensible, e.g. doe not use wildcard imports.  We would also
> > want to make one large commit to format all existing code before turning
> > this on.
> >
> > Mark - Thank you for the information about the existing setup.
> >
> > Mark, Darrel, Kevin - Given all of your comments, I think it might make
> > more sense to add the flag to enable it in Travis CI rather than as part
> > of  the build.  This way your build pass regardless of whitespace, but
> the
> > CI job would fail on PRs if they did not adhere to the standard
> formatting.
> >
> > Anthony - It doesn’t seem to me that turning this on would have the
> effect
> > of combining reformatting commits and logic changes.  Rather, since all
> > code would already be formatted, there would no longer be any
> reformatting
> > commits except for single large commits when the code style file was
> > updated.
> >
> > > On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt  >
> > wrote:
> > >
> > > I like the idea of doing this but I don't think Checkstyle should be
> > enabled until all of the code is reformatted.
> > >
> > > Also, last time I checked there was still a problem with the IntelliJ
> > auto-format settings.  It still used wildcard imports, which I believe we
> > don't allow.  I've manually changed my settings in Editor->Code
> Style->Java
> > to "Use single class import" to correct that problem.  I couldn't see how
> > to get Gradle to do it.
> > >
> > >
> > > Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :
> > >> Source code with a consistent look-and-feel makes it easier for people
> > to join the project community and contribute.
> > >>
> > >> Let’s continue to keep reformatting commits separate from logic
> > changes—otherwise it’s too hard to review.
> > >>
> > >> Anthony
> > >>
> > >>> On Oct 12, 2016, at 10:06 AM, Dan Smith  wrote:
> > >>>
> > >>> +1
> > >>>
> > >>> This might be a good time to reformat the code since I don't think
> > there
> > >>> are too many long lived feature branches outstanding.
> > >>>
> > >>> -Dan
> > >>>
> > >>> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart  >
> > wrote:
> > >>>
> >  I would like to advocate for adding a Checkstyle  .
> >  sourceforge.net/> or Spotless  >
> >  gradle task to our build process to ensure that all code checked in
> > meets
> >  the formatting standards described on the wiki <
> > https://cwiki.apache.org/
> >  confluence/display/GEODE/Code+Style+Guide> (and in the
> > intellij/eclipse
> >  formatter xml files in our repository). This will alleviate
> > difficulties
> >  reviewing code when whitespace or formatting has changed since all
> > code
> >  checked in will already comply with standards.
> > >
> >
> >
>

Re: Coding practices/standards

[Darrel Schneider]

If Travis CI is only run on pull requests then that is not enough because
committers do not submit pull requests. Having it run during the gradle
build or precheckin target is also needed. In addition to that I also
wanted PRs to be checked.


On Wed, Oct 12, 2016 at 11:12 AM, Jared Stewart  wrote:

> It would certainly be necessary to make sure that the code style to be
> enforced is sensible, e.g. doe not use wildcard imports.  We would also
> want to make one large commit to format all existing code before turning
> this on.
>
> Mark - Thank you for the information about the existing setup.
>
> Mark, Darrel, Kevin - Given all of your comments, I think it might make
> more sense to add the flag to enable it in Travis CI rather than as part
> of  the build.  This way your build pass regardless of whitespace, but the
> CI job would fail on PRs if they did not adhere to the standard formatting.
>
> Anthony - It doesn’t seem to me that turning this on would have the effect
> of combining reformatting commits and logic changes.  Rather, since all
> code would already be formatted, there would no longer be any reformatting
> commits except for single large commits when the code style file was
> updated.
>
> > On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt 
> wrote:
> >
> > I like the idea of doing this but I don't think Checkstyle should be
> enabled until all of the code is reformatted.
> >
> > Also, last time I checked there was still a problem with the IntelliJ
> auto-format settings.  It still used wildcard imports, which I believe we
> don't allow.  I've manually changed my settings in Editor->Code Style->Java
> to "Use single class import" to correct that problem.  I couldn't see how
> to get Gradle to do it.
> >
> >
> > Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :
> >> Source code with a consistent look-and-feel makes it easier for people
> to join the project community and contribute.
> >>
> >> Let’s continue to keep reformatting commits separate from logic
> changes—otherwise it’s too hard to review.
> >>
> >> Anthony
> >>
> >>> On Oct 12, 2016, at 10:06 AM, Dan Smith  wrote:
> >>>
> >>> +1
> >>>
> >>> This might be a good time to reformat the code since I don't think
> there
> >>> are too many long lived feature branches outstanding.
> >>>
> >>> -Dan
> >>>
> >>> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart 
> wrote:
> >>>
>  I would like to advocate for adding a Checkstyle   sourceforge.net/> or Spotless 
>  gradle task to our build process to ensure that all code checked in
> meets
>  the formatting standards described on the wiki <
> https://cwiki.apache.org/
>  confluence/display/GEODE/Code+Style+Guide> (and in the
> intellij/eclipse
>  formatter xml files in our repository). This will alleviate
> difficulties
>  reviewing code when whitespace or formatting has changed since all
> code
>  checked in will already comply with standards.
> >
>
>

Re: Coding practices/standards

[Jared Stewart]

It would certainly be necessary to make sure that the code style to be enforced 
is sensible, e.g. doe not use wildcard imports.  We would also want to make one 
large commit to format all existing code before turning this on.

Mark - Thank you for the information about the existing setup.

Mark, Darrel, Kevin - Given all of your comments, I think it might make more 
sense to add the flag to enable it in Travis CI rather than as part of  the 
build.  This way your build pass regardless of whitespace, but the CI job would 
fail on PRs if they did not adhere to the standard formatting.

Anthony - It doesn’t seem to me that turning this on would have the effect of 
combining reformatting commits and logic changes.  Rather, since all code would 
already be formatted, there would no longer be any reformatting commits except 
for single large commits when the code style file was updated.

> On Oct 12, 2016, at 11:01 AM, Bruce Schuchardt  wrote:
> 
> I like the idea of doing this but I don't think Checkstyle should be enabled 
> until all of the code is reformatted.
> 
> Also, last time I checked there was still a problem with the IntelliJ 
> auto-format settings.  It still used wildcard imports, which I believe we 
> don't allow.  I've manually changed my settings in Editor->Code Style->Java 
> to "Use single class import" to correct that problem.  I couldn't see how to 
> get Gradle to do it.
> 
> 
> Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :
>> Source code with a consistent look-and-feel makes it easier for people to 
>> join the project community and contribute.
>> 
>> Let’s continue to keep reformatting commits separate from logic 
>> changes—otherwise it’s too hard to review.
>> 
>> Anthony
>> 
>>> On Oct 12, 2016, at 10:06 AM, Dan Smith  wrote:
>>> 
>>> +1
>>> 
>>> This might be a good time to reformat the code since I don't think there
>>> are too many long lived feature branches outstanding.
>>> 
>>> -Dan
>>> 
>>> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart  wrote:
>>> 
 I would like to advocate for adding a Checkstyle  or Spotless 
 gradle task to our build process to ensure that all code checked in meets
 the formatting standards described on the wiki  (and in the intellij/eclipse
 formatter xml files in our repository). This will alleviate difficulties
 reviewing code when whitespace or formatting has changed since all code
 checked in will already comply with standards.
> 

Re: Coding practices/standards

[Bruce Schuchardt]

I like the idea of doing this but I don't think Checkstyle should be 
enabled until all of the code is reformatted.


Also, last time I checked there was still a problem with the IntelliJ 
auto-format settings.  It still used wildcard imports, which I believe 
we don't allow.  I've manually changed my settings in Editor->Code 
Style->Java to "Use single class import" to correct that problem.  I 
couldn't see how to get Gradle to do it.



Le 10/12/2016 à 10:28 AM, Anthony Baker a écrit :

Source code with a consistent look-and-feel makes it easier for people to join 
the project community and contribute.

Let’s continue to keep reformatting commits separate from logic 
changes—otherwise it’s too hard to review.

Anthony


On Oct 12, 2016, at 10:06 AM, Dan Smith  wrote:

+1

This might be a good time to reformat the code since I don't think there
are too many long lived feature branches outstanding.

-Dan

On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart  wrote:


I would like to advocate for adding a Checkstyle  or Spotless 
gradle task to our build process to ensure that all code checked in meets
the formatting standards described on the wiki  (and in the intellij/eclipse
formatter xml files in our repository). This will alleviate difficulties
reviewing code when whitespace or formatting has changed since all code
checked in will already comply with standards.

Re: Coding practices/standards

[Anthony Baker]

Source code with a consistent look-and-feel makes it easier for people to join 
the project community and contribute.

Let’s continue to keep reformatting commits separate from logic 
changes—otherwise it’s too hard to review.

Anthony

> On Oct 12, 2016, at 10:06 AM, Dan Smith  wrote:
> 
> +1
> 
> This might be a good time to reformat the code since I don't think there
> are too many long lived feature branches outstanding.
> 
> -Dan
> 
> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart  wrote:
> 
>> I would like to advocate for adding a Checkstyle > sourceforge.net/> or Spotless 
>> gradle task to our build process to ensure that all code checked in meets
>> the formatting standards described on the wiki > confluence/display/GEODE/Code+Style+Guide> (and in the intellij/eclipse
>> formatter xml files in our repository). This will alleviate difficulties
>> reviewing code when whitespace or formatting has changed since all code
>> checked in will already comply with standards.



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: Coding practices/standards

[Mark Bretl]

Hi Jared,

The Checkstyle plugin is already added to the Gradle build. By default it
is turned off, however, if you pass the property '-PstaticAnalysis' to the
command-line, it will run CheckStyle and FindBugs. It was setup with only
basic configuration, so the small amount of work would be to add our own
rule filters to the task,. The plugin definition is in the
gradle/code-analysis.gradle file.

Kevin,

This is exactly why I implemented it the way that I did originally. If
people want to be proactive and run it, then it can be turned on, otherwise
it will most likely 'fail the build' due to issues.

--Mark

--Mark

On Wed, Oct 12, 2016 at 10:12 AM, Kevin Duling  wrote:

> I know I've been guilty of submitting code for review that hasn't matched
> the code style in the past.  As a result, I've checked the "reformat code"
> box on the 'commit' dialog in IDEA, pointing to the IntelliJ xml formatter.
>
> It's frustrating to me that this sort of thing is still an issue in
> software development.  Ever since I've started my career, there's been
> arguments about whitespace; how far to indent and what line the curly brace
> goes on.  And I'm yet to be convinced one way is superior to the other.
> What is easier for one to read is more difficult for another.
>
> While I'm developing and doing frequent local builds, I do not want a build
> to fail due to whitespace issues.
>
> So I am in favor of adding this if there is an option to disable like we
> can disable javadoc and tests, but enforce it by default and/or precheckin.
>
>
> On Wed, Oct 12, 2016 at 10:07 AM, Nabarun Nag  wrote:
>
> > +1
> >
> > On Wed, Oct 12, 2016 at 10:06 AM Dan Smith  wrote:
> >
> > > +1
> > >
> > > This might be a good time to reformat the code since I don't think
> there
> > > are too many long lived feature branches outstanding.
> > >
> > > -Dan
> > >
> > > On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart 
> > > wrote:
> > >
> > > > I would like to advocate for adding a Checkstyle  > > > sourceforge.net/> or Spotless 
> > > > gradle task to our build process to ensure that all code checked in
> > meets
> > > > the formatting standards described on the wiki <
> > > https://cwiki.apache.org/
> > > > confluence/display/GEODE/Code+Style+Guide> (and in the
> > intellij/eclipse
> > > > formatter xml files in our repository). This will alleviate
> > difficulties
> > > > reviewing code when whitespace or formatting has changed since all
> code
> > > > checked in will already comply with standards.
> > >
> >
>

Re: Coding practices/standards

[Kevin Duling]

I know I've been guilty of submitting code for review that hasn't matched
the code style in the past.  As a result, I've checked the "reformat code"
box on the 'commit' dialog in IDEA, pointing to the IntelliJ xml formatter.

It's frustrating to me that this sort of thing is still an issue in
software development.  Ever since I've started my career, there's been
arguments about whitespace; how far to indent and what line the curly brace
goes on.  And I'm yet to be convinced one way is superior to the other.
What is easier for one to read is more difficult for another.

While I'm developing and doing frequent local builds, I do not want a build
to fail due to whitespace issues.

So I am in favor of adding this if there is an option to disable like we
can disable javadoc and tests, but enforce it by default and/or precheckin.


On Wed, Oct 12, 2016 at 10:07 AM, Nabarun Nag  wrote:

> +1
>
> On Wed, Oct 12, 2016 at 10:06 AM Dan Smith  wrote:
>
> > +1
> >
> > This might be a good time to reformat the code since I don't think there
> > are too many long lived feature branches outstanding.
> >
> > -Dan
> >
> > On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart 
> > wrote:
> >
> > > I would like to advocate for adding a Checkstyle  > > sourceforge.net/> or Spotless 
> > > gradle task to our build process to ensure that all code checked in
> meets
> > > the formatting standards described on the wiki <
> > https://cwiki.apache.org/
> > > confluence/display/GEODE/Code+Style+Guide> (and in the
> intellij/eclipse
> > > formatter xml files in our repository). This will alleviate
> difficulties
> > > reviewing code when whitespace or formatting has changed since all code
> > > checked in will already comply with standards.
> >
>

Re: Coding practices/standards

[Darrel Schneider]

+1
When a pull request is submitted some type of build is automatically done
to make sure the code compiles. Would this new "code check" also be done in
that automatic check?


On Wed, Oct 12, 2016 at 10:07 AM, Nabarun Nag  wrote:

> +1
>
> On Wed, Oct 12, 2016 at 10:06 AM Dan Smith  wrote:
>
> > +1
> >
> > This might be a good time to reformat the code since I don't think there
> > are too many long lived feature branches outstanding.
> >
> > -Dan
> >
> > On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart 
> > wrote:
> >
> > > I would like to advocate for adding a Checkstyle  > > sourceforge.net/> or Spotless 
> > > gradle task to our build process to ensure that all code checked in
> meets
> > > the formatting standards described on the wiki <
> > https://cwiki.apache.org/
> > > confluence/display/GEODE/Code+Style+Guide> (and in the
> intellij/eclipse
> > > formatter xml files in our repository). This will alleviate
> difficulties
> > > reviewing code when whitespace or formatting has changed since all code
> > > checked in will already comply with standards.
> >
>

Re: Coding practices/standards

[Nabarun Nag]

+1

On Wed, Oct 12, 2016 at 10:06 AM Dan Smith  wrote:

> +1
>
> This might be a good time to reformat the code since I don't think there
> are too many long lived feature branches outstanding.
>
> -Dan
>
> On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart 
> wrote:
>
> > I would like to advocate for adding a Checkstyle  > sourceforge.net/> or Spotless 
> > gradle task to our build process to ensure that all code checked in meets
> > the formatting standards described on the wiki <
> https://cwiki.apache.org/
> > confluence/display/GEODE/Code+Style+Guide> (and in the intellij/eclipse
> > formatter xml files in our repository). This will alleviate difficulties
> > reviewing code when whitespace or formatting has changed since all code
> > checked in will already comply with standards.
>

Re: Coding practices/standards

[Dan Smith]

+1

This might be a good time to reformat the code since I don't think there
are too many long lived feature branches outstanding.

-Dan

On Wed, Oct 12, 2016 at 10:00 AM, Jared Stewart  wrote:

> I would like to advocate for adding a Checkstyle  sourceforge.net/> or Spotless 
> gradle task to our build process to ensure that all code checked in meets
> the formatting standards described on the wiki  confluence/display/GEODE/Code+Style+Guide> (and in the intellij/eclipse
> formatter xml files in our repository). This will alleviate difficulties
> reviewing code when whitespace or formatting has changed since all code
> checked in will already comply with standards.