Re: more spotless problems on Windows

[Jared Stewart]

@Naba

I pushed the proposed changes to a branch here: 
https://github.com/jaredjstewart/incubator-geode/tree/windowsLF 
<https://github.com/jaredjstewart/incubator-geode/tree/windowsLF>

Can you see if you still have the same problem when you check out this branch?

> On Nov 4, 2016, at 12:07 PM, Bruce Schuchardt <bschucha...@pivotal.io> wrote:
> 
> Udo and I tried that & it didn't work.  Maybe you'll have better luck.
> 
> Le 11/4/2016 à 11:59 AM, Jared Stewart a écrit :
>> From the spotless devs:
>> 
>> Git is not a pure content store, it mucks with line endings. Regardless of 
>> what you check-in, it will store your files with unix line endings in the 
>> repo.
>> 
>> Then, when you checkout, it will modify the line endings to suit your 
>> platform. Unless you add a .gitattributes file to tell git "forget the 
>> platform, do what this file says".
>> 
>> Remove lineEndings 'UNIX' in your build.gradle [Jared - we should put 
>> ‘GIT_ATTRIBUTES’ in its place], and add a .gitattributes file in your root 
>> directory with the content * text eol=lf and your problem will be fixed.
>> 
>> 
>> 
>>> On Nov 4, 2016, at 10:48 AM, Nabarun Nag <n...@pivotal.io> wrote:
>>> 
>>> Thank you Jared. I wanted to confirm that this was not an isolated incident
>>> specific to my machine.
>>> 
>>> Regards
>>> Naba
>>> 
>>> On Fri, Nov 4, 2016 at 10:45 AM Jared Stewart <jstew...@pivotal.io> wrote:
>>> 
>>>> @Naba,
>>>> 
>>>> I filed a bug report with Spotless this morning.  The Spotless devs have
>>>> been very responsive so far in my experience, hopefully this will be fixed
>>>> soon.
>>>> 
>>>>> On Nov 4, 2016, at 10:35 AM, Nabarun Nag <n...@pivotal.io> wrote:
>>>>> 
>>>>> @Udo, I confirmed that this is not limited to my windows 10 environment.
>>>> I
>>>>> ran  the steps on a Windows Server 2016 AMI instance and the same error
>>>>> occurred in the AMI too. [source checkout time 4th Nov 10:00AM PST]
>>>>> 
>>>>> I wanted to know if there is a mandate on what the value of
>>>>> core.autocrlf should
>>>>> be set to on a windows machine for geode dev work. For my experiments
>>>> value
>>>>> of core.autocrlf was set to true. [recommended for cross platform
>>>>> development]
>>>>> 
>>>>> Regards
>>>>> Naba
>>>>> 
>>>>> 
>>>>> On Thu, Nov 3, 2016 at 10:18 PM Nabarun Nag <n...@pivotal.io> wrote:
>>>>> 
>>>>>> @Jared
>>>>>> I ran ./gradlew spotlessApply on the Windows 10 machine using git bash
>>>>>> this is what has happened.
>>>>>> NOTE: I started the below steps on a fresh git clone of the open side.
>>>>>> [Steps:
>>>>>> 1.  git clone
>>>> https://git-wip-us.apache.org/repos/asf/incubator-geode.git
>>>>>> open
>>>>>> 2. cd open
>>>>>> 3. git checkout -b develop origin/develop]
>>>>>> 
>>>>>> *Step 1. ./gradlew clean build -Dskip.tests=true*
>>>>>> 
>>>>>> FAILURE: Build failed with an exception.
>>>>>> 
>>>>>> * What went wrong:
>>>>>> Execution failed for task ':geode-core:spotlessJavaCheck'.
>>>>>>> Format violations were found. Run 'gradlew spotlessApply' to fix them.
>>>>>> 
>>>> geode-core\src\main\java\org\apache\geode\internal\statistics\StatArchiveReader.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\cache\query\dunit\PdxLocalQueryVersionedClassDUnitTest.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\internal\cache\execute\ClientServerFunctionExecutionDUnitTest.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\internal\cache\functions\TestFunction.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\internal\statistics\StatArchiveWithMissingResourceTypeRegressionTest.java
>>>>>> 
>>>>>> *Step 2: ./gradlew spotlessApply*
>>>>>> 
>>>>>> BUILD SUCCESSFUL
>>>>>> 
>>>>>> Total time: 12.728 secs
>>>>>> 
>>>>>> 
>>>>>> *Step 3: git status*
>>>

Re: more spotless problems on Windows

[Jared Stewart]

Correction:
“ * text eol=lf”
> On Nov 4, 2016, at 12:09 PM, Jared Stewart <jstew...@pivotal.io> wrote:
> 
> “ *test eol=lf”

Re: more spotless problems on Windows

[Jared Stewart]

@Bruce

To be clear, are you saying you did both of the following?

1). Create a .gitattributes file with “ *test eol=lf” at the top level of Geode
2). Change ‘UNIX’ in build.gradle to ‘GIT_ATTRIBUTES'

> On Nov 4, 2016, at 12:07 PM, Bruce Schuchardt <bschucha...@pivotal.io> wrote:
> 
> Udo and I tried that & it didn't work.  Maybe you'll have better luck.
> 
> Le 11/4/2016 à 11:59 AM, Jared Stewart a écrit :
>> From the spotless devs:
>> 
>> Git is not a pure content store, it mucks with line endings. Regardless of 
>> what you check-in, it will store your files with unix line endings in the 
>> repo.
>> 
>> Then, when you checkout, it will modify the line endings to suit your 
>> platform. Unless you add a .gitattributes file to tell git "forget the 
>> platform, do what this file says".
>> 
>> Remove lineEndings 'UNIX' in your build.gradle [Jared - we should put 
>> ‘GIT_ATTRIBUTES’ in its place], and add a .gitattributes file in your root 
>> directory with the content * text eol=lf and your problem will be fixed.
>> 
>> 
>> 
>>> On Nov 4, 2016, at 10:48 AM, Nabarun Nag <n...@pivotal.io> wrote:
>>> 
>>> Thank you Jared. I wanted to confirm that this was not an isolated incident
>>> specific to my machine.
>>> 
>>> Regards
>>> Naba
>>> 
>>> On Fri, Nov 4, 2016 at 10:45 AM Jared Stewart <jstew...@pivotal.io> wrote:
>>> 
>>>> @Naba,
>>>> 
>>>> I filed a bug report with Spotless this morning.  The Spotless devs have
>>>> been very responsive so far in my experience, hopefully this will be fixed
>>>> soon.
>>>> 
>>>>> On Nov 4, 2016, at 10:35 AM, Nabarun Nag <n...@pivotal.io> wrote:
>>>>> 
>>>>> @Udo, I confirmed that this is not limited to my windows 10 environment.
>>>> I
>>>>> ran  the steps on a Windows Server 2016 AMI instance and the same error
>>>>> occurred in the AMI too. [source checkout time 4th Nov 10:00AM PST]
>>>>> 
>>>>> I wanted to know if there is a mandate on what the value of
>>>>> core.autocrlf should
>>>>> be set to on a windows machine for geode dev work. For my experiments
>>>> value
>>>>> of core.autocrlf was set to true. [recommended for cross platform
>>>>> development]
>>>>> 
>>>>> Regards
>>>>> Naba
>>>>> 
>>>>> 
>>>>> On Thu, Nov 3, 2016 at 10:18 PM Nabarun Nag <n...@pivotal.io> wrote:
>>>>> 
>>>>>> @Jared
>>>>>> I ran ./gradlew spotlessApply on the Windows 10 machine using git bash
>>>>>> this is what has happened.
>>>>>> NOTE: I started the below steps on a fresh git clone of the open side.
>>>>>> [Steps:
>>>>>> 1.  git clone
>>>> https://git-wip-us.apache.org/repos/asf/incubator-geode.git
>>>>>> open
>>>>>> 2. cd open
>>>>>> 3. git checkout -b develop origin/develop]
>>>>>> 
>>>>>> *Step 1. ./gradlew clean build -Dskip.tests=true*
>>>>>> 
>>>>>> FAILURE: Build failed with an exception.
>>>>>> 
>>>>>> * What went wrong:
>>>>>> Execution failed for task ':geode-core:spotlessJavaCheck'.
>>>>>>> Format violations were found. Run 'gradlew spotlessApply' to fix them.
>>>>>> 
>>>> geode-core\src\main\java\org\apache\geode\internal\statistics\StatArchiveReader.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\cache\query\dunit\PdxLocalQueryVersionedClassDUnitTest.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\internal\cache\execute\ClientServerFunctionExecutionDUnitTest.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\internal\cache\functions\TestFunction.java
>>>>>> 
>>>> geode-core\src\test\java\org\apache\geode\internal\statistics\StatArchiveWithMissingResourceTypeRegressionTest.java
>>>>>> 
>>>>>> *Step 2: ./gradlew spotlessApply*
>>>>>> 
>>>>>> BUILD SUCCESSFUL
>>>>>> 
>>>>>> Total time: 12.728 secs
>>>>>> 
>>>>>> 
>>>>>> *Step 3: git status*
>>>>>> 
>>>>>> modified:
&

Re: more spotless problems on Windows

[Jared Stewart]

From the spotless devs:

Git is not a pure content store, it mucks with line endings. Regardless of what 
you check-in, it will store your files with unix line endings in the repo.

Then, when you checkout, it will modify the line endings to suit your platform. 
Unless you add a .gitattributes file to tell git "forget the platform, do what 
this file says".

Remove lineEndings 'UNIX' in your build.gradle [Jared - we should put 
‘GIT_ATTRIBUTES’ in its place], and add a .gitattributes file in your root 
directory with the content * text eol=lf and your problem will be fixed.



> On Nov 4, 2016, at 10:48 AM, Nabarun Nag <n...@pivotal.io> wrote:
> 
> Thank you Jared. I wanted to confirm that this was not an isolated incident
> specific to my machine.
> 
> Regards
> Naba
> 
> On Fri, Nov 4, 2016 at 10:45 AM Jared Stewart <jstew...@pivotal.io> wrote:
> 
>> @Naba,
>> 
>> I filed a bug report with Spotless this morning.  The Spotless devs have
>> been very responsive so far in my experience, hopefully this will be fixed
>> soon.
>> 
>>> On Nov 4, 2016, at 10:35 AM, Nabarun Nag <n...@pivotal.io> wrote:
>>> 
>>> @Udo, I confirmed that this is not limited to my windows 10 environment.
>> I
>>> ran  the steps on a Windows Server 2016 AMI instance and the same error
>>> occurred in the AMI too. [source checkout time 4th Nov 10:00AM PST]
>>> 
>>> I wanted to know if there is a mandate on what the value of
>>> core.autocrlf should
>>> be set to on a windows machine for geode dev work. For my experiments
>> value
>>> of core.autocrlf was set to true. [recommended for cross platform
>>> development]
>>> 
>>> Regards
>>> Naba
>>> 
>>> 
>>> On Thu, Nov 3, 2016 at 10:18 PM Nabarun Nag <n...@pivotal.io> wrote:
>>> 
>>>> @Jared
>>>> I ran ./gradlew spotlessApply on the Windows 10 machine using git bash
>>>> this is what has happened.
>>>> NOTE: I started the below steps on a fresh git clone of the open side.
>>>> [Steps:
>>>> 1.  git clone
>> https://git-wip-us.apache.org/repos/asf/incubator-geode.git
>>>> open
>>>> 2. cd open
>>>> 3. git checkout -b develop origin/develop]
>>>> 
>>>> *Step 1. ./gradlew clean build -Dskip.tests=true*
>>>> 
>>>> FAILURE: Build failed with an exception.
>>>> 
>>>> * What went wrong:
>>>> Execution failed for task ':geode-core:spotlessJavaCheck'.
>>>>> Format violations were found. Run 'gradlew spotlessApply' to fix them.
>>>> 
>>>> 
>> geode-core\src\main\java\org\apache\geode\internal\statistics\StatArchiveReader.java
>>>> 
>>>> 
>> geode-core\src\test\java\org\apache\geode\cache\query\dunit\PdxLocalQueryVersionedClassDUnitTest.java
>>>> 
>>>> 
>> geode-core\src\test\java\org\apache\geode\internal\cache\execute\ClientServerFunctionExecutionDUnitTest.java
>>>> 
>>>> 
>> geode-core\src\test\java\org\apache\geode\internal\cache\functions\TestFunction.java
>>>> 
>>>> 
>> geode-core\src\test\java\org\apache\geode\internal\statistics\StatArchiveWithMissingResourceTypeRegressionTest.java
>>>> 
>>>> 
>>>> *Step 2: ./gradlew spotlessApply*
>>>> 
>>>> BUILD SUCCESSFUL
>>>> 
>>>> Total time: 12.728 secs
>>>> 
>>>> 
>>>> *Step 3: git status*
>>>> 
>>>> modified:
>>>> 
>> geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java
>>>>   modified:
>>>> 
>> geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxLocalQueryVersionedClassDUnitTest.java
>>>>   modified:
>>>> 
>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java
>>>>   modified:
>>>> 
>> geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java
>>>>   modified:
>>>> 
>> geode-core/src/test/java/org/apache/geode/internal/statistics/StatArchiveWithMissingResourceTypeRegressionTest.java
>>>> 
>>>> 
>>>> *Step 4 : git add .*
>>>> warning: LF will be replaced by CRLF in
>>>> 
>> geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java.
>>>> The file will have its original line endings in your working direct

Re: Gfsh Parsing

[Jared Stewart]

Huge +1 to the idea of simplifying Gfsh parsing.  I find the green help text 
from Spring Shell too be less readable then the black text from JoptSimple, but 
I assume we can configure that to our choosing.

> On Nov 4, 2016, at 10:05 AM, Jinmei Liao  wrote:
> 
> This is a good idea. I'll reach out to find it out. Thanks!
> 
> On Fri, Nov 4, 2016 at 9:48 AM, Michael Stolz  wrote:
> 
>> Can we suggest improvements to the Spring help capabilities? The Spring
>> community tends to be very responsive to good suggestions.
>> 
>> --
>> Mike Stolz
>> Principal Engineer - Gemfire Product Manager
>> Mobile: 631-835-4771
>> On Nov 4, 2016 8:27 AM, "Jinmei Liao"  wrote:
>> 
>>> We have several jira issues related to gfsh parsing (GEODE-1598,
>>> GEODE-1912). After spending some time understanding how the parsing
>> works,
>>> I found out we have three components intertwined together all trying to
>> do
>>> parsing: Gfsh, JoptSimple and Spring Shell. I started an experiment by
>>> getting rid of Gfsh and JoptSimple parsing and only using Spring Shell.
>> The
>>> outcome is I am able to maintain the current parsing and tabbing
>> completion
>>> capabilities (and fix a few bugs) by removing 40+ files. The only
>>> difference I see so far lies in the help and hint messages. It seems the
>>> main reason we are using these home backed Gfsh parsing is to provide
>> more
>>> readable help messages. Below are the differences:
>>> 
>>> Using Spring Shell's provided help:
>>> 
>>> Using Gfsh Parsing with JoptSimple:
>>> 
>>> 
>>> ​
>>> ​I do like the outcome of the latter, but added complexity of the code is
>>> too expensive to bear. So I am asking the community how important it is
>> to
>>> maintain the current style of help? Can we do with the cheaper way by
>> just
>>> using whatever provided by the libraries?
>>> 
>>> --
>>> Cheers
>>> 
>>> Jinmei
>>> 
>> 
> 
> 
> 
> -- 
> Cheers
> 
> Jinmei

Re: more spotless problems on Windows

[Jared Stewart]

@Naba,  

I filed a bug report with Spotless this morning.  The Spotless devs have been 
very responsive so far in my experience, hopefully this will be fixed soon.

> On Nov 4, 2016, at 10:35 AM, Nabarun Nag <n...@pivotal.io> wrote:
> 
> @Udo, I confirmed that this is not limited to my windows 10 environment. I
> ran  the steps on a Windows Server 2016 AMI instance and the same error
> occurred in the AMI too. [source checkout time 4th Nov 10:00AM PST]
> 
> I wanted to know if there is a mandate on what the value of
> core.autocrlf should
> be set to on a windows machine for geode dev work. For my experiments value
> of core.autocrlf was set to true. [recommended for cross platform
> development]
> 
> Regards
> Naba
> 
> 
> On Thu, Nov 3, 2016 at 10:18 PM Nabarun Nag <n...@pivotal.io> wrote:
> 
>> @Jared
>> I ran ./gradlew spotlessApply on the Windows 10 machine using git bash
>> this is what has happened.
>> NOTE: I started the below steps on a fresh git clone of the open side.
>> [Steps:
>> 1.  git clone https://git-wip-us.apache.org/repos/asf/incubator-geode.git
>> open
>> 2. cd open
>> 3. git checkout -b develop origin/develop]
>> 
>> *Step 1. ./gradlew clean build -Dskip.tests=true*
>> 
>> FAILURE: Build failed with an exception.
>> 
>> * What went wrong:
>> Execution failed for task ':geode-core:spotlessJavaCheck'.
>>> Format violations were found. Run 'gradlew spotlessApply' to fix them.
>> 
>> geode-core\src\main\java\org\apache\geode\internal\statistics\StatArchiveReader.java
>> 
>> geode-core\src\test\java\org\apache\geode\cache\query\dunit\PdxLocalQueryVersionedClassDUnitTest.java
>> 
>> geode-core\src\test\java\org\apache\geode\internal\cache\execute\ClientServerFunctionExecutionDUnitTest.java
>> 
>> geode-core\src\test\java\org\apache\geode\internal\cache\functions\TestFunction.java
>> 
>> geode-core\src\test\java\org\apache\geode\internal\statistics\StatArchiveWithMissingResourceTypeRegressionTest.java
>> 
>> 
>> *Step 2: ./gradlew spotlessApply*
>> 
>> BUILD SUCCESSFUL
>> 
>> Total time: 12.728 secs
>> 
>> 
>> *Step 3: git status*
>> 
>> modified:
>> geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java
>>modified:
>> geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxLocalQueryVersionedClassDUnitTest.java
>>modified:
>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java
>>modified:
>> geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java
>>modified:
>> geode-core/src/test/java/org/apache/geode/internal/statistics/StatArchiveWithMissingResourceTypeRegressionTest.java
>> 
>> 
>> *Step 4 : git add .*
>> warning: LF will be replaced by CRLF in
>> geode-core/src/main/java/org/apache/geode/internal/statistics/StatArchiveReader.java.
>> The file will have its original line endings in your working directory.
>> warning: LF will be replaced by CRLF in
>> geode-core/src/test/java/org/apache/geode/cache/query/dunit/PdxLocalQueryVersionedClassDUnitTest.java.
>> The file will have its original line endings in your working directory.
>> warning: LF will be replaced by CRLF in
>> geode-core/src/test/java/org/apache/geode/internal/cache/execute/ClientServerFunctionExecutionDUnitTest.java.
>> The file will have its original line endings in your working directory.
>> warning: LF will be replaced by CRLF in
>> geode-core/src/test/java/org/apache/geode/internal/cache/functions/TestFunction.java.
>> The file will have its original line endings in your working directory.
>> warning: LF will be replaced by CRLF in
>> geode-core/src/test/java/org/apache/geode/internal/statistics/StatArchiveWithMissingResourceTypeRegressionTest.java.
>> The file will have its original line endings in your working directory.
>> 
>> 
>> *Step 5: git status*
>> On branch develop
>> Your branch is up-to-date with 'origin/develop'.
>> nothing to commit, working tree clean
>> 
>> 
>> *Step 6: ./gradlew clean build -Dskip.tests=true*
>> BUILD SUCCESSFUL
>> 
>> Total time: 5 mins 28.64 secs
>> 
>> NOTE: This happens only the first time. I did run the above steps,couple
>> of times on  fresh checkouts and I was able to reproduce it every time.
>> 
>> However, after running spotlessApply and git add . the first time, the
>> spotless errors do not reoccur on subsequent builds.
>> 
>> I will try ru

Re: more spotless problems on Windows

[Jared Stewart]

The only Windows machine I have is running Windows 8, and I am unable to 
reproduce this on that machine.  I don’t think .gitattributes would affect 
this, since we have already configured spotless to always use Unix line 
endings.  

Naba - Can you run ‘./gradlew spotlessApply’ and push the results to a branch 
so I can see what Spotless was complaining about?

> On Nov 3, 2016, at 4:09 PM, Udo Kohlmeyer  wrote:
> 
> I think we seriously have to look at using .gitattributes for this...
> 
> As I initially said, it should be a no brainer.. it should just automatically 
> just work.
> 
> --Udo
> 
> 
> On 4/11/16 9:00 am, Bruce Schuchardt wrote:
>> It's been working on my Windows 7 machine under a cygwin shell.   I just ran 
>> it again using "clean bulid -Dskip.tests=true" from the root Geode directory 
>> on the develop branch.
>> 
>> Run spotlessApply and let us know how it modified the files.
>> 
>> 
>> Le 11/3/2016 à 12:38 PM, Nabarun Nag a écrit :
>>> I tested gradlew build on a windows 10 machine to test the spotless feature.
>>> 
>>> Steps:
>>> 1.  git clone https://git-wip-us.apache.org/repos/asf/incubator-geode.git
>>> open
>>> 2. cd open
>>> 3. git checkout -b develop origin/develop
>>> 4.  ./gradlew clean build -Dskip.tests=true
>>> 
>>> The build failed with multiple formatting error on each file.
>>> 
>>> In my opinion the issue still exists. It will be awesome if someone else
>>> can verify if the issue still exists by running the build steps on a
>>> different windows machine.
>>> 
>>> Regards
>>> Nabarun
>>> 
>>> On Mon, Oct 24, 2016 at 3:50 PM Bruce Schuchardt 
>>> wrote:
>>> 
 The lineEndings setting works great. I've pushed the change to develop
 
 On Mon, Oct 24, 2016 at 3:47 PM, Dan Smith  wrote:
 
> I think we have a fix for the spotless line ending issue on windows;
 Bruce
> will check it in shortly:
> 
> diff --git a/build.gradle b/build.gradle
> index a734e05..6e82433 100755
> --- a/build.gradle
> +++ b/build.gradle
> @@ -88,6 +88,7 @@ subprojects {
> 
>apply plugin: "com.diffplug.gradle.spotless"
>spotless {
> +lineEndings = 'unix';
>  java {
>eclipseFormatFile
> "${rootProject.projectDir}/etc/eclipse-java-google-style.xml"
> 
> 
> On Mon, Oct 24, 2016 at 2:50 PM, Bruce Schuchardt <
 bschucha...@pivotal.io>
> wrote:
> 
>> Running geode-core:spotlessCheck complains that all of the .java files
>> have format violations
>> 
>> * What went wrong:
>> Execution failed for task ':geode-core:spotlessJavaCheck'.
>>> Format violations were found. Run 'gradlew spotlessApply' to fix
 them.
>> geode-core\src\jca\java\org\apache\geode\internal\ra\GFConne
>> ctionFactoryImpl.java
>> geode-core\src\jca\java\org\apache\geode\internal\ra\
> GFConnectionImpl.java
>> geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
>> LocalTransaction.java
>> geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
>> ManagedConnection.java
>> geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
>> ManagedConnectionFactory.java
>> geode-core\src\jca\java\org\apache\geode\internal\ra\spi\JCA
>> ManagedConnectionnMetaData.java
>> etc.
>> 
>> Until this is fixed I can't validate that the changes I check in
 conform
>> to the formatting rules.
>> 
>> 
> 

Re: jmh benchmarks

[Jared Stewart]

+1 Jmh is great

On Nov 2, 2016 11:18 AM, "Dan Smith"  wrote:

> Hi all,
>
> I'd like to add some support for running benchmarks with jmh to geode. Is
> this something we're interested in having? JMH is a framework for easily
> writing microbenchmarks. It's probably not that useful for large scale
> multiple member benchmarks, but it can help us benchmark and optimize
> smaller pieces of code.
>
> You can look at this review request for what I want to change and a couple
> of example benchmarks:
>
> https://reviews.apache.org/r/53395/
>
> -Dan
>

Re: Tweaking the IntelliJ and Eclipse formatters

[Jared Stewart]

1) +0
2) +1
3) +1
4) +1 

- Jared 

> On Oct 27, 2016, at 3:11 PM, Kirk Lund  wrote:
> 
> I'd like to propose making a few changes to our IntelliJ and Eclipse
> formatters as well as the Eclipse importorder (all in etc/):
> 
> 1) increase max line length (100 is way too short)
> 2) make (hopefully minor) changes to make the two formatters more
> consistent with each other
> 3) change Eclipse importorder to follow the google style as closely as
> possible
> 4) update the import ordering in IntelliJ formatter to match Eclipse
> 
> The goal is to make both formatters produce the exact same results
> including ordering of imports while maintaining them to be as close to the
> google style as possible. Right now if you run IntelliJ formatter, the
> result will fail the spotlessCheck. We may have to make some small
> compromises in our adherence to the google style but I think that's
> reasonable in order to get the formatters both working consistently.
> 
> The gradle spotless tasks currently use the Eclipse formatter. One further
> change would be to add the Eclipse importorder file for spotless to use.
> 
> -Kirk

Re: [DISCUSS] Graduation

[Jared Stewart]

+1

On Oct 25, 2016 5:58 PM, "Dan Smith"  wrote:

> +1
>
> -Dan
>
> On Tue, Oct 25, 2016 at 5:43 PM, Joey McAllister 
> wrote:
>
> > +1
> >
> > On Tue, Oct 25, 2016 at 5:42 PM Anthony Baker  wrote:
> >
> > > +1
> > >
> > > > On Oct 25, 2016, at 5:25 PM, Roman Shaposhnik 
> wrote:
> > > >
> > > > Unless somebody objects strongly to my #2 and #3 proposals I'm going
> > > > to kick of this thread on private@.
> > >
> > >
> >
>

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 <dsm...@pivotal.io> 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 <jstew...@pivotal.io> wrote:
> 
>> Try this commit hash instead: d0175ec5aa8acf1b34ece3183fe03e9874450cbb
>> (from feature/spotlessPlugin).
>> 
>> 
>>> On Oct 21, 2016, at 4:16 PM, Kirk Lund <kl...@pivotal.io> 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 <jstew...@pivotal.io>
>> 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 <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io>
>>>> wrote:
>>>>> 
>>>>>> Done! :)
>>>>>> 
>>>>>> - Jared
>>>>>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com>
>> 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 <asf.mbr...@gmail.com>
>>>> 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

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 <kl...@pivotal.io> 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 <jstew...@pivotal.io> 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 <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io>
>> wrote:
>>> 
>>>> Done! :)
>>>> 
>>>> - Jared
>>>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com> 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 <asf.mbr...@gmail.com>
>> 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 <kh...@pivotal.io>
>>>> 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
>>>>>>

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 
<https://github.com/apache/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 <kl...@pivotal.io> 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 <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io>
>> wrote:
>> 
>>> Done! :)
>>> 
>>> - Jared
>>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com> 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 <asf.mbr...@gmail.com>
>> 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 <kh...@pivotal.io>
>>> 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.
>>>>&

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 <kl...@pivotal.io> 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 <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io>
>> wrote:
>> 
>>> Done! :)
>>> 
>>> - Jared
>>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com> 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 <asf.mbr...@gmail.com>
>> 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 <kh...@pivotal.io>
>>> 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
>>>>>>>>>> 
>>>>&

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 <asf.mbr...@gmail.com> 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 <jstew...@pivotal.io> wrote:
> 
>> Done! :)
>> 
>> - Jared
>>> On Oct 21, 2016, at 12:27 PM, Mark Bretl <asf.mbr...@gmail.com> 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 <asf.mbr...@gmail.com> 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 <kh...@pivotal.io>
>> 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 <kl...@apache.org>
>> 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 Styl

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 <asf.mbr...@gmail.com> 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 <kh...@pivotal.io> 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 <kl...@apache.org> 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 <kl...@pivotal.io> 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 <dsm...@pivotal.io> 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/
>>>

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 <dsm...@pivotal.io> 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 <dsm...@pivotal.io> 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 >

Re: Hosting the docs (was Re: [VOTE] Release Apache Geode (incubating) 1.0.0-incubating - RC2)

[Jared Stewart]

Sadly just looked at the license for wkhtmltopdf and it uses GPL 3.0.  I 
believe that would be an issue as discussed here 
<https://www.apache.org/licenses/GPL-compatibility.html>.

> On Oct 18, 2016, at 1:38 PM, Jared Stewart <jstew...@pivotal.io> wrote:
> 
> In the past I have used wkhtmltopdf to build programmatically PDFs from HTML 
> documents.  We could try using this to generate a PDF version of the docs in 
> the interim until we can generate a PDF directly from book binder.
> 
>> On Oct 18, 2016, at 1:09 PM, Michael Stolz <mst...@pivotal.io> wrote:
>> 
>> Its really important to ship pdf docs because its very difficult to search
>> otherwise.
>> 
>> --
>> Mike Stolz
>> Principal Engineer, GemFire Product Manager
>> Mobile: 631-835-4771
>> 
>> On Tue, Oct 18, 2016 at 12:49 PM, Joey McAllister <jmcallis...@pivotal.io>
>> wrote:
>> 
>>> @Dan: I didn't realize there was a docs link in the top-level README. We
>>> can change that for the next release, and I can look into redirecting
>>> geode.docs.pivotal.io to geode.incubator.apache.org/docs/ in the meantime,
>>> once the docs are posted there..
>>> 
>>> @William: We don't currently have a PDF for these docs. I'm researching
>>> options.
>>> 
>>> On Mon, Oct 17, 2016 at 6:53 PM William Markito <wmark...@pivotal.io>
>>> wrote:
>>> 
>>> IHMO, it would be really nice to ship a PDF version of the docs.
>>> 
>>> About the examples, if we could package and ship sources only for that
>>> module, that would be cool as well.
>>> 
>>> 
>>> 
>>> On Mon, Oct 17, 2016 at 5:21 PM, Dan Smith <dsm...@pivotal.io> wrote:
>>> 
>>>> Along these lines, should we distributing the docs with the binary
>>> release?
>>>> Or maybe just providing a link to them? The README.md shipped with
>>> 1.0.RC2
>>>> points to http://geode.docs.pivotal.io/ .
>>>> 
>>>> What about geode-examples? Should that be part of the binary release?
>>>> 
>>>> -Dan
>>>> 
>>>> On Mon, Oct 17, 2016 at 2:21 PM, Joey McAllister <jmcallis...@pivotal.io
>>>> 
>>>> wrote:
>>>> 
>>>>> @Roman: Nothing that I can think of, apart from giving the community
>>> time
>>>>> to offer feedback here (which, it looks like, is all positive). William
>>>>> Markito and I were able to build and test a local version of the
>>> website
>>>>> with the docs included.
>>>>> 
>>>>> Based on the +1s here, I'd like to go ahead and push the current docs
>>> to
>>>>> the website. I'll also document the process in a README.
>>>>> 
>>>>> On Mon, Oct 17, 2016 at 12:49 PM Roman Shaposhnik <
>>> ro...@shaposhnik.org>
>>>>> wrote:
>>>>> 
>>>>>> On Mon, Oct 17, 2016 at 10:57 AM, Anthony Baker <aba...@pivotal.io>
>>>>> wrote:
>>>>>>> Since the geode docs have now been merged to the develop branch,
>>>> let’s
>>>>>> start
>>>>>>> hosting them on http://geode.apache.org.  Thoughts?
>>>>>> 
>>>>>> Huge +1! Anything stopping you from pushing the first update and
>>> start
>>>>>> maintaining
>>>>>> it a'la Hadoop:
>>>>>>   https://hadoop.apache.org/docs/
>>>>>> ?
>>>>>> 
>>>>>> Thanks,
>>>>>> Roman.
>>>>>> 
>>>>> 
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> 
>>> ~/William
>>> 
> 

Re: Hosting the docs (was Re: [VOTE] Release Apache Geode (incubating) 1.0.0-incubating - RC2)

[Jared Stewart]

In the past I have used wkhtmltopdf to build programmatically PDFs from HTML 
documents.  We could try using this to generate a PDF version of the docs in 
the interim until we can generate a PDF directly from book binder.

> On Oct 18, 2016, at 1:09 PM, Michael Stolz  wrote:
> 
> Its really important to ship pdf docs because its very difficult to search
> otherwise.
> 
> --
> Mike Stolz
> Principal Engineer, GemFire Product Manager
> Mobile: 631-835-4771
> 
> On Tue, Oct 18, 2016 at 12:49 PM, Joey McAllister 
> wrote:
> 
>> @Dan: I didn't realize there was a docs link in the top-level README. We
>> can change that for the next release, and I can look into redirecting
>> geode.docs.pivotal.io to geode.incubator.apache.org/docs/ in the meantime,
>> once the docs are posted there..
>> 
>> @William: We don't currently have a PDF for these docs. I'm researching
>> options.
>> 
>> On Mon, Oct 17, 2016 at 6:53 PM William Markito 
>> wrote:
>> 
>> IHMO, it would be really nice to ship a PDF version of the docs.
>> 
>> About the examples, if we could package and ship sources only for that
>> module, that would be cool as well.
>> 
>> 
>> 
>> On Mon, Oct 17, 2016 at 5:21 PM, Dan Smith  wrote:
>> 
>>> Along these lines, should we distributing the docs with the binary
>> release?
>>> Or maybe just providing a link to them? The README.md shipped with
>> 1.0.RC2
>>> points to http://geode.docs.pivotal.io/ .
>>> 
>>> What about geode-examples? Should that be part of the binary release?
>>> 
>>> -Dan
>>> 
>>> On Mon, Oct 17, 2016 at 2:21 PM, Joey McAllister >> 
>>> wrote:
>>> 
 @Roman: Nothing that I can think of, apart from giving the community
>> time
 to offer feedback here (which, it looks like, is all positive). William
 Markito and I were able to build and test a local version of the
>> website
 with the docs included.
 
 Based on the +1s here, I'd like to go ahead and push the current docs
>> to
 the website. I'll also document the process in a README.
 
 On Mon, Oct 17, 2016 at 12:49 PM Roman Shaposhnik <
>> ro...@shaposhnik.org>
 wrote:
 
> On Mon, Oct 17, 2016 at 10:57 AM, Anthony Baker 
 wrote:
>> Since the geode docs have now been merged to the develop branch,
>>> let’s
> start
>> hosting them on http://geode.apache.org.  Thoughts?
> 
> Huge +1! Anything stopping you from pushing the first update and
>> start
> maintaining
> it a'la Hadoop:
>https://hadoop.apache.org/docs/
> ?
> 
> Thanks,
> Roman.
> 
 
>>> 
>> 
>> 
>> 
>> --
>> 
>> ~/William
>> 

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52889/#review152719
---


Ship it!




Ship It!

- Jared Stewart


On Oct. 14, 2016, 6:41 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> ---
> 
> (Updated Oct. 14, 2016, 6:41 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  34fd5a994c5a69d64398de3cb867892a30827e2c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  def17920b198e88f358dfcf4025f4a3eee6fd0df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  4d1bae904c4964ca2be713fd8450f2f5f6db2552 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JMXConnectionConfiguration.java
>  4f57baa85e3b5cd084ffd33c61f7741d20b0fdc9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
>  c544e6fe0f8d3c1975775979d1354c4adc7cf87a 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JsonAuthorizationCacheStartRule.java
>  136319c06d9bd1e46b452219bf2894cf969fc1f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java
>  1377fb643d035ad911ee5ceb80b87dfd72855428 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java
>  4beff0bd55d69601266a0a856c8dcb80ffd4e1f7 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanServerConnectionRule.java
>  92430327e999af627e88956fc24aa1592e9a000d 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ManagerMBeanAuthorizationJUnitTest.java
>  873b6491f6703378d87383d96dc35e299b19a3fa 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java
>  e5cbd15da9274d2a86be399f45c9239b2ae373be 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/ResourcePermi

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jared Stewart]

> On Oct. 14, 2016, 5:21 p.m., Jared Stewart wrote:
> > Many usages of ServerStarter have to specify 
> > "org/apache/geode/management/internal/security/clientServer.json".  I think 
> > it would be convenient to have some static factory methods like 
> > ServerStarter.startWithSecurity() that specify a default set of Properties 
> > like SampleSecurityManager and 
> > "org/apache/geode/management/internal/security/clientServer.json". 
> > 
> > The same addition would be useful in CacheServerStartupRule.  It would be 
> > convenient to not have to pass in 
> > "org/apache/geode/management/internal/security/cacheServer.json".
> > 
> > RestSecurityService has authorize methods that show up as unused in 
> > Intellij.  I know from looking at the other changes that they are used by 
> > @PreAuthorized annotations in the CrudControllers, but it might be nice to 
> > add comments to these methods explaining where they're used so that people 
> > in the future don't think they can be deleted.
> 
> Jinmei Liao wrote:
> The thing is, that json file is very test specific. Each test can use a 
> different json file to change the username/password it's using and 
> permissions granted to that user. We started the tests using this 
> SampleSecurityManager which unfortunatly requires the json file. I would hope 
> all our later tests would use SimpleSecurityManager instead to get rid this.

Right now it looked like every place using this was specifying that same json 
file.  Creating a static factory method like 
CacheServerStartupRule.withSampleSecurityManager(Properties properties) that 
would return a CacheServerStartupRule with the sample security manager and 
security json specified by default, but still allowing the user to override 
them if they want to seems like it would give the best of both worlds.


- Jared


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52889/#review152694
---


On Oct. 14, 2016, 5:25 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> ---
> 
> (Updated Oct. 14, 2016, 5:25 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/securi

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jared Stewart]

My mistake, thank you for the info. Your comment led me here 
<http://blog.beanbaginc.com/2015/01/26/an-effective-rbtools-workflow-for-git/>, 
and I think RBTools will soothe the pain I had downloads and manually applying 
diffs.

> On Oct 14, 2016, at 11:13 AM, Anthony Baker <aba...@pivotal.io> wrote:
> 
> Review board requests can be updated and you can see the changes between 
> diffs.
> 
> Anthony
> 
>> On Oct 14, 2016, at 10:42 AM, Jared Stewart <jstew...@pivotal.io 
>> <mailto:jstew...@pivotal.io>> wrote:
>> 
>> If we used github PRs instead of review board, your second set of changes 
>> could show up in this same review request!  
>> 
>> 
>> - Jared
> 

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jared Stewart]

> On Oct. 14, 2016, 5:12 p.m., Kirk Lund wrote:
> > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java,
> >  line 66
> > <https://reviews.apache.org/r/52889/diff/1/?file=1538108#file1538108line66>
> >
> > This is still a DistributedTest if it uses the dunit VMs. This uses a 
> > dunit rule so it probably still uses dunit VMs.
> 
> Jinmei Liao wrote:
> This test uses the ServerStarter rule now. I just pushed another set of 
> change that adds more javadoc to the ServerStarter and LocatorStarter rule. 
> Basically these twos rules starts up server/locator in the current VM. It's 
> not using dunit VMs. This makes this test runs a lot faster. But you are 
> right, the test itself should not be called a DunitTest anymore. I'll fix 
> that.

If we used github PRs instead of review board, your second set of changes could 
show up in this same review request!  Do we have the option to do that for our 
team?  I also prefer the interface for github and checking out a remote branch 
is easier than applying a diff.  (For example, with a diff I have to manually 
make sure to apply it to the correct base.)


- Jared


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52889/#review152695
---


On Oct. 14, 2016, 5:25 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> -------
> 
> (Updated Oct. 14, 2016, 5:25 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  34fd5a994c5a69d64398de3cb867892a30827e2c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  def17920b198e88f358dfcf4025f4a3eee6fd0df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  4d1bae904c4964ca2be713fd8450f2f5f6db2552 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JMXConnectionConfiguration.java
>  4f57baa85e3b5cd084ffd33c61f7741d20b0fdc9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
>  c544e6fe0f8d3c1975775979d1354c4adc7cf87a 
>   
> geode-core/src/test/java/org/apache/geod

Re: Review Request 52889: GEODE-1993: refactor tests to use rules rather than abstract classes

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52889/#review152694
---



Many usages of ServerStarter have to specify 
"org/apache/geode/management/internal/security/clientServer.json".  I think it 
would be convenient to have some static factory methods like 
ServerStarter.startWithSecurity() that specify a default set of Properties like 
SampleSecurityManager and 
"org/apache/geode/management/internal/security/clientServer.json". 

The same addition would be useful in CacheServerStartupRule.  It would be 
convenient to not have to pass in 
"org/apache/geode/management/internal/security/cacheServer.json".

RestSecurityService has authorize methods that show up as unused in Intellij.  
I know from looking at the other changes that they are used by @PreAuthorized 
annotations in the CrudControllers, but it might be nice to add comments to 
these methods explaining where they're used so that people in the future don't 
think they can be deleted.

- Jared Stewart


On Oct. 14, 2016, 4:19 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52889/
> ---
> 
> (Updated Oct. 14, 2016, 4:19 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> * created ServerStarter and LocatorStarter in the rule package
> * refacterred LocatorServerConfigurationRule
> * refactor tests to use these rules
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  59e00c8bd0a7f2759f579abf99a87fcd98b42a06 
>   
> geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
>  79b70f875f74c15eb041e9d1cbd5b1f8ceeda899 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java
>  c22fff3655131cf908a54289b66faf84311c5c69 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  388094840fc587e662d231783fd5c7c8faf7b485 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  7bbfbcc9691000601cf6bf4dd0be7c7394cf3fcf 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java
>  721a4312456e0f1eaf41a6c1f41016cfe56450e4 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerStartupRule.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java
>  84155a9d82292dc70c6412908a1b57a09c1aea98 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java
>  0084cb8f5471ce7ecab086c955a842766d6ed790 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  750ce2ac76ecd0860f5f678a22615697c9ea5009 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  b64a6f7b1c63eb41f5823b4971d5041431748db9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  9acf8dbd172cca27ba4da942b3d5c1bd0368ff83 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java
>  34fd5a994c5a69d64398de3cb867892a30827e2c 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java
>  def17920b198e88f358dfcf4025f4a3eee6fd0df 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/GfshShellConnectionRule.java
>  4d1bae904c4964ca2be713fd8450f2f5f6db2552 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JMXConnectionConfiguration.java
>  4f57baa85e3b5cd084ffd33c61f7741d20b0fdc9 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java
>  c544e6fe0f8d3c1975775979d1354c4adc7cf87a 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/JsonAuthorizationCacheStartRule.java
>  136319c06d9bd1e46b452219bf2894cf969fc1f1 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java
>  1377fb643d035ad911ee5c

Re: Build failed in Jenkins: Geode-nightly #621

[Jared Stewart]

Thank you, I will add a hook so this doesn’t happen again.

- Jared 
> On Oct 13, 2016, at 10:16 AM, Dan Smith <dsm...@pivotal.io> wrote:
> 
> Yet one more reason to run ./gradlew build *before* pushing changes to
> develop
> 
> You can automate this by adding a pre-push trigger to your git repo. This
> will only do the push if the build succeeds:
> 
> echo "./gradlew build" > .git/hooks/pre-push
> chmod u+x .git/hooks/pre-push
> 
> You can do git push --no-verify to skip the check if you want to.
> 
> -Dan
> 
> On Thu, Oct 13, 2016 at 9:07 AM, Jared Stewart <jstew...@pivotal.io> wrote:
> 
>> This file was accidentally committed in GEODE-999. Jinmei removed it from
>> develop this morning after we saw the RAT failed last night.  I haven’t
>> figured out yet what generated the file in the first place.
>> 
>>> On Oct 13, 2016, at 9:04 AM, Anthony Baker <aba...@pivotal.io> wrote:
>>> 
>>> Anyone know why this file was generated by the build?  It shows up in
>> the rat report.
>>> 
>>> !? /home/jenkins/jenkins-slave/workspace/Geode-nightly/
>> artifacts-jstewartgeode999/test/gemfire-jstewartgeode999-files.tgz
>>> 
>>> Anthony
>>> 
>>> 
>>>> Begin forwarded message:
>>>> 
>>>> From: Apache Jenkins Server <jenk...@builds.apache.org>
>>>> Subject: Build failed in Jenkins: Geode-nightly #621
>>>> Date: October 13, 2016 at 8:36:21 AM PDT
>>>> To: dev@geode.incubator.apache.org, jil...@pivotal.io, n...@pivotal.io
>>>> Reply-To: dev@geode.incubator.apache.org
>>>> 
>>>> See <https://builds.apache.org/job/Geode-nightly/621/changes>
>>>> 
>>>> Changes:
>>>> 
>>>> [jiliao] GEODE-1986: correctly set the flag indicating if cluster
>> configuration
>>>> 
>>>> [jiliao] GEODE-1979: refactor SecurityClusterConfig to remove the
>> flakiness.
>>>> 
>>>> [jiliao] GEODE-999: Converted from Firefox driver to PhantomJS driver
>> to run
>>>> 
>>>> [jiliao] GEODE-1966: Unauthorized users cannot access pulseVersion
>> details
>>>> 
>>>> [jiliao] GEODE-1532: Fix Pulse Clickjacking vuln.
>>>> 
>>>> [nnag] GEODE-1978: Slowing down the receivers
>>>> 
>>>> --
>>>> [...truncated 407 lines...]
>>>> :geode-rebalancer:jar
>>>> :geode-rebalancer:javadoc
>>>> :geode-rebalancer:javadocJar
>>>> :geode-rebalancer:sourcesJar
>>>> :geode-rebalancer:signArchives SKIPPED
>>>> :geode-wan:jar
>>>> :geode-wan:javadoc
>>>> :geode-wan:javadocJar
>>>> :geode-wan:sourcesJar
>>>> :geode-wan:signArchives SKIPPED
>>>> :geode-web:javadoc UP-TO-DATE
>>>> :geode-web:javadocJar
>>>> :geode-web:sourcesJar
>>>> :geode-web:war
>>>> :geode-web:signArchives SKIPPED
>>>> :geode-web-api:javadoc<https://builds.apache.org/job/Geode-
>> nightly/ws/geode-web-api/src/main/java/org/apache/geode/
>> rest/internal/web/controllers/CommonCrudController.java>:196: warning -
>> @return tag has no arguments.
>>>> 
>>>> 1 warning
>>>> :geode-web-api:javadocJar
>>>> :geode-web-api:sourcesJar
>>>> :geode-web-api:war
>>>> :geode-web-api:signArchives SKIPPED
>>>> :geode-assembly:distTar
>>>> :geode-assembly:distZip
>>>> :geode-assembly:writeBuildInfo
>>>> :geode-assembly:srcDistTar
>>>> :geode-assembly:srcDistZip
>>>> :geode-assembly:signArchives SKIPPED
>>>> :geode-assembly:assemble
>>>> :geode-assembly:compileTestJavaNote: Some input files use or override
>> a deprecated API.
>>>> Note: Recompile with -Xlint:deprecation for details.
>>>> Note: Some input files use unchecked or unsafe operations.
>>>> Note: Recompile with -Xlint:unchecked for details.
>>>> 
>>>> :geode-assembly:processTestResources
>>>> :geode-assembly:testClasses
>>>> :geode-assembly:checkMissedTests
>>>> :geode-assembly:installDist
>>>> :geode-assembly:test
>>>> :geode-assembly:distributedTest
>>>> :geode-assembly:flakyTest
>>>> :geode-assembly:integrationTest
>>>> :geode-common:assemble
>>>> :geode-common:compileTestJava
>>>> :geode-common:processTestResources UP-TO-D

Re: Coding practices/standards

[Jared Stewart]

ription, 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",
> +
> +};
> 
> 
> 
> 
> 
> On Thu, Oct 13, 2016 at 9:51 AM, Jared Stewart <jstew...@pivotal.io> 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 <kdul...@pivotal.io> 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 <jstew...@pivotal.io>
>>>> 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 <kh...@pivotal.io> 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 <jstew...@pivotal.io>
>>>> 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
&g

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 <kdul...@pivotal.io> 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 <jstew...@pivotal.io>
>> 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 <kh...@pivotal.io> 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 <jstew...@pivotal.io>
>> 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 lo

Re: Build failed in Jenkins: Geode-nightly #621

[Jared Stewart]

This file was accidentally committed in GEODE-999. Jinmei removed it from 
develop this morning after we saw the RAT failed last night.  I haven’t figured 
out yet what generated the file in the first place.

> On Oct 13, 2016, at 9:04 AM, Anthony Baker  wrote:
> 
> Anyone know why this file was generated by the build?  It shows up in the rat 
> report.
> 
> !? 
> /home/jenkins/jenkins-slave/workspace/Geode-nightly/artifacts-jstewartgeode999/test/gemfire-jstewartgeode999-files.tgz
> 
> Anthony
> 
> 
>> Begin forwarded message:
>> 
>> From: Apache Jenkins Server 
>> Subject: Build failed in Jenkins: Geode-nightly #621
>> Date: October 13, 2016 at 8:36:21 AM PDT
>> To: dev@geode.incubator.apache.org, jil...@pivotal.io, n...@pivotal.io
>> Reply-To: dev@geode.incubator.apache.org
>> 
>> See 
>> 
>> Changes:
>> 
>> [jiliao] GEODE-1986: correctly set the flag indicating if cluster 
>> configuration
>> 
>> [jiliao] GEODE-1979: refactor SecurityClusterConfig to remove the flakiness.
>> 
>> [jiliao] GEODE-999: Converted from Firefox driver to PhantomJS driver to run
>> 
>> [jiliao] GEODE-1966: Unauthorized users cannot access pulseVersion details
>> 
>> [jiliao] GEODE-1532: Fix Pulse Clickjacking vuln.
>> 
>> [nnag] GEODE-1978: Slowing down the receivers
>> 
>> --
>> [...truncated 407 lines...]
>> :geode-rebalancer:jar
>> :geode-rebalancer:javadoc
>> :geode-rebalancer:javadocJar
>> :geode-rebalancer:sourcesJar
>> :geode-rebalancer:signArchives SKIPPED
>> :geode-wan:jar
>> :geode-wan:javadoc
>> :geode-wan:javadocJar
>> :geode-wan:sourcesJar
>> :geode-wan:signArchives SKIPPED
>> :geode-web:javadoc UP-TO-DATE
>> :geode-web:javadocJar
>> :geode-web:sourcesJar
>> :geode-web:war
>> :geode-web:signArchives SKIPPED
>> :geode-web-api:javadoc:196:
>>  warning - @return tag has no arguments.
>> 
>> 1 warning
>> :geode-web-api:javadocJar
>> :geode-web-api:sourcesJar
>> :geode-web-api:war
>> :geode-web-api:signArchives SKIPPED
>> :geode-assembly:distTar
>> :geode-assembly:distZip
>> :geode-assembly:writeBuildInfo
>> :geode-assembly:srcDistTar
>> :geode-assembly:srcDistZip
>> :geode-assembly:signArchives SKIPPED
>> :geode-assembly:assemble
>> :geode-assembly:compileTestJavaNote: Some input files use or override a 
>> deprecated API.
>> Note: Recompile with -Xlint:deprecation for details.
>> Note: Some input files use unchecked or unsafe operations.
>> Note: Recompile with -Xlint:unchecked for details.
>> 
>> :geode-assembly:processTestResources
>> :geode-assembly:testClasses
>> :geode-assembly:checkMissedTests
>> :geode-assembly:installDist
>> :geode-assembly:test
>> :geode-assembly:distributedTest
>> :geode-assembly:flakyTest
>> :geode-assembly:integrationTest
>> :geode-common:assemble
>> :geode-common:compileTestJava
>> :geode-common:processTestResources UP-TO-DATE
>> :geode-common:testClasses
>> :geode-common:checkMissedTests
>> :geode-common:test
>> :geode-common:distributedTest
>> :geode-common:flakyTest
>> :geode-common:integrationTest
>> :geode-core:assemble
>> :geode-core:checkMissedTests
>> :geode-core:test
>> :geode-core:distributedTest
>> :geode-core:flakyTest
>> :geode-core:integrationTest
>> :geode-cq:assemble
>> :geode-cq:compileTestJavaNote: Some input files use or override a deprecated 
>> API.
>> Note: Recompile with -Xlint:deprecation for details.
>> Note: Some input files use unchecked or unsafe operations.
>> Note: Recompile with -Xlint:unchecked for details.
>> 
>> :geode-cq:processTestResources
>> :geode-cq:testClasses
>> :geode-cq:checkMissedTests
>> :geode-cq:test
>> :geode-cq:distributedTest
>> :geode-cq:flakyTest
>> :geode-cq:integrationTest
>> :geode-json:assemble
>> :geode-json:compileTestJava UP-TO-DATE
>> :geode-json:processTestResources UP-TO-DATE
>> :geode-json:testClasses UP-TO-DATE
>> :geode-json:checkMissedTests UP-TO-DATE
>> :geode-json:test UP-TO-DATE
>> :geode-json:distributedTest UP-TO-DATE
>> :geode-json:flakyTest UP-TO-DATE
>> :geode-json:integrationTest UP-TO-DATE
>> :geode-junit:javadoc
>> :geode-junit:javadocJar
>> :geode-junit:sourcesJar
>> :geode-junit:signArchives SKIPPED
>> :geode-junit:assemble
>> :geode-junit:compileTestJava
>> :geode-junit:processTestResources UP-TO-DATE
>> :geode-junit:testClasses
>> :geode-junit:checkMissedTests
>> :geode-junit:test
>> :geode-junit:distributedTest
>> :geode-junit:flakyTest
>> :geode-junit:integrationTest
>> :geode-lucene:assemble
>> :geode-lucene:compileTestJavaNote: Some input files use or override a 
>> deprecated API.
>> Note: Recompile with -Xlint:deprecation for details.
>> Note: Some input files use unchecked or unsafe operations.
>> Note: Recompile with -Xlint:unchecked for details.
>> 
>> :geode-lucene:processTestResources
>> 

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 <kh...@pivotal.io> 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 <jstew...@pivotal.io> 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

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

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

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" <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 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 <dsm...@pivotal.io> 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 <https://github.com/diffplug/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 <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 <dsm...@pivotal.io> 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 <https://github.com/diffplug/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.
> 

Coding practices/standards

[Jared Stewart]

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: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151724
---


Ship it!




Ship It!

- Jared Stewart


On Oct. 6, 2016, 6:34 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 6, 2016, 6:34 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> * update the NOTICE files
> 
> 
> Diffs
> -
> 
>   geode-assembly/src/main/dist/NOTICE 
> 1924007f6c4822f59e0a830acf4e2b01f67170ef 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/META-INF/NOTICE 
> 40f9fc875d6bc53d073061495738f636ad60433c 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   geode-web-api/build.gradle 3ea652b65456168689250f35be553c2eeda193d9 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/META-INF/NOTICE 
> 2f4da6e85a2daa75e136bbc80e62768193f65db9 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/META-INF/NOTICE 
> dd46891056a99365494ac212fec8ef8b3dd0a201 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151676
---



My previous issues were due to applying this patch to DEVELOP rather than 
release/1.0.0-incubating.

- Jared Stewart


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151584
---



I also have a few unit tests failing from this patch: 
```
org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest > 
testInitFile_TwoBadCommands FAILED
java.lang.AssertionError: Log records written expected:<4> but was:<3>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at 
org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest.testInitFile_TwoBadCommands(GfshInitFileJUnitTest.java:402)

org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest > 
testInitFile_OneBadCommand FAILED
java.lang.AssertionError: Log records written expected:<4> but was:<3>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at 
org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest.testInitFile_OneBadCommand(GfshInitFileJUnitTest.java:371)

org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest > 
testInitFile_BadAndGoodCommands FAILED
java.lang.AssertionError: Log records written expected:<4> but was:<3>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at 
org.apache.geode.management.internal.cli.shell.GfshInitFileJUnitTest.testInitFile_BadAndGoodCommands(GfshInitFileJUnitTest.java:434)
```

- Jared Stewart


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>  

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jared Stewart]

> On Oct. 5, 2016, 9:18 p.m., Jared Stewart wrote:
> > Aren't the (OutputStream) casts in JSONUtils.java redundant since 
> > outputStream here is already an instance of HeapDataOutputStream which 
> > extends OutputStream?
> > 
> > Also, I believe our schemaLocations in *-servlet.xml ought to refer to 
> > versioned URLs so that we do not break when a new Spring version is 
> > released before we have updated to that version.
> 
> Kevin Duling wrote:
> When I was trying to do this ticket, I ran in to the necessity of having 
> to cast because the compiler objected, saying there was no matching method 
> that would accept it -- even though it clearly appeared there was.  By 
> casting, it resolved it.  I'm sure Jinmei ran in to the same issue when she 
> worked on the upgrade.
> 
> Jinmei Liao wrote:
> If I remeber correctly, if I don't cast, it complains about some method 
> ambiguity.
> 
> And for schemeLocations, It is recommended to use the "versionless" XSDs, 
> because they're mapped to the current version of the framework you're using 
> in your application. 
> 
> http://stackoverflow.com/questions/20894695/spring-configuration-xml-schema-with-or-without-version

Cool, thanks for the info about the "versionless" XSDs!


- Jared


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151560
---


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> -------
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jared Stewart]

> On Oct. 5, 2016, 9:47 p.m., Jared Stewart wrote:
> > I also get compilation errors in PulseAppListener and 
> > ExceptionHandlingAdvice when I apply this patch, even after refreshing 
> > gradle.
> 
> Jinmei Liao wrote:
> Hnnn, do a clean and refresh. I am not getting the errors.

No luck.  Have you tried doing a clean and then running "gradle uiTest"?

I verified in gradle that this is a problem caused by the removal of the 
"force" block in dependency-resolution.gradle.

Before this patch:
```
~/projects/gemfire/open (develop) $ gradle :geode-pulse:dI --dependency 
spring-web
:buildSrc:compileJava UP-TO-DATE
:buildSrc:compileGroovy UP-TO-DATE
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar UP-TO-DATE
:buildSrc:assemble UP-TO-DATE
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:compileTestGroovy UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build UP-TO-DATE
:geode-pulse:dependencyInsight
org.springframework:spring-web:4.2.4.RELEASE (forced)

org.springframework:spring-web:3.0.7.RELEASE -> 4.2.4.RELEASE
--- org.springframework.security:spring-security-web:3.1.7.RELEASE
 --- compile

BUILD SUCCESSFUL

```

After this patch:
```
~/projects/gemfire/open (develop) $ gradle :geode-pulse:dI --dependency 
spring-web
:buildSrc:compileJava UP-TO-DATE
:buildSrc:compileGroovy UP-TO-DATE
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar UP-TO-DATE
:buildSrc:assemble UP-TO-DATE
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:compileTestGroovy UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build UP-TO-DATE
:geode-pulse:dependencyInsight
org.springframework:spring-web:3.0.7.RELEASE
--- org.springframework.security:spring-security-web:3.1.7.RELEASE
 --- compile

BUILD SUCCESSFUL

```


- Jared


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151569
---


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/w

Re: Review Request 52571: GEODE-1570: upgrade spring libraries

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52571/#review151560
---



Aren't the (OutputStream) casts in JSONUtils.java redundant since outputStream 
here is already an instance of HeapDataOutputStream which extends OutputStream?

Also, I believe our schemaLocations in *-servlet.xml ought to refer to 
versioned URLs so that we do not break when a new Spring version is released 
before we have updated to that version.

- Jared Stewart


On Oct. 5, 2016, 9:06 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52571/
> ---
> 
> (Updated Oct. 5, 2016, 9:06 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Kevin Duling, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1570: upgrade spring libraries and related libraries
> 
> * upgrade the spring related libraries
> * rafactored the tests to get rid of redundant code
> 
> 
> Diffs
> -
> 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestInterfaceJUnitTest.java
>  0d9351889b54b3ffdd6cba4e7ade4ae5e5a17093 
>   
> geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityDUnitTest.java
>  a9d90ed5489a9ab6b936674369b5e5a870651738 
>   geode-assembly/src/test/resources/expected_jars.txt 
> 939464a92a3f1846b8fb9b9d1faa75dad5133289 
>   geode-core/build.gradle 3cbdfbea41a7f34b237394e92a463919d0109f47 
>   
> geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshInitFileJUnitTest.java
>  9e7b7bf17920a9dc42905bab4ccb4590ca20223a 
>   
> geode-core/src/test/java/org/apache/geode/security/AbstractSecureServerDUnitTest.java
>  435b426627deea6c67ecd79bc014ec97774e2ebf 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthentication.java
>  425f5a5aa76c8219678164ebbb012d1361519bc7 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/security/GemFireAuthenticationProvider.java
>  f4575ccf5c894e2cbaaebc51ffc4d7f56fce68c4 
>   
> geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/service/MemberGatewayHubService.java
>  51abb3f39902938149fb67a416f571ba31b75d77 
>   geode-pulse/src/main/webapp/Login.html 
> f22490f4df098c0b12dadeefcb1855a51efc6281 
>   geode-pulse/src/main/webapp/WEB-INF/mvc-dispatcher-servlet.xml 
> cb7181d3279e2c2dfc86ddc201fa6dc002eb55fe 
>   geode-pulse/src/main/webapp/WEB-INF/spring-security.xml 
> 924dd50f579a6e4e306b94f4a7fd88e87f978add 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAuthTest.java
>  a77e0cacdcde69f47fbd13c588601072c432ee0d 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAutomatedTest.java
>  a4f14f89ea739e7390565d652fc5affa6e1326c6 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JSONUtils.java
>  ccf3b9d9f0d0aebadbf628ef6bdcef1084e81b5d 
>   
> geode-web-api/src/main/java/org/apache/geode/rest/internal/web/util/JsonWriter.java
>  1fcffbd4eee26fee890f158794727647145a1914 
>   geode-web-api/src/main/webapp/WEB-INF/geode-servlet.xml 
> c75d975c4313f953cce5ac7f5a8cd8a228718a8c 
>   geode-web/src/main/webapp/WEB-INF/geode-mgmt-servlet.xml 
> 3b1d1658ea2414190852cc1268fd9f1beccf4d20 
>   gradle/dependency-resolution.gradle 
> 91d1755848ba9ce23fab3190555c2906bcffd97b 
>   gradle/dependency-versions.properties 
> 65fd2ee1d46156781ee70165aac578c5ca14490a 
> 
> Diff: https://reviews.apache.org/r/52571/diff/
> 
> 
> Testing
> ---
> 
> precheckin running.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52488: GEODE-420: fix Pulse test when not using any SSLConfig

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52488/#review151256
---


Ship it!




Ship It!

- Jared Stewart


On Oct. 3, 2016, 11:20 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52488/
> ---
> 
> (Updated Oct. 3, 2016, 11:20 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-420: fix Pulse test when not using any SSLConfig
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
>  089dbacb056da84d6d6ceb36dad6f09b9ce73890 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java
>  a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java
>  5024250f8ff3867e0f527d7ef5fbbd501e2d64ee 
> 
> Diff: https://reviews.apache.org/r/52488/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: Review Request 52488: GEODE-420: fix Pulse test when not using any SSLConfig

[Jared Stewart]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52488/#review151196
---


Ship it!




Ship It!

- Jared Stewart


On Oct. 3, 2016, 5:52 p.m., Jinmei Liao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52488/
> ---
> 
> (Updated Oct. 3, 2016, 5:52 p.m.)
> 
> 
> Review request for geode, Jared Stewart, Kevin Duling, Kirk Lund, and Udo 
> Kohlmeyer.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-420: fix Pulse test when not using any SSLConfig
> 
> 
> Diffs
> -
> 
>   
> geode-core/src/main/java/org/apache/geode/management/internal/JettyHelper.java
>  089dbacb056da84d6d6ceb36dad6f09b9ce73890 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/testbed/driver/PulseUITest.java
>  a2365a2dfa3e221fa50664c3f8d96fae2bd93c86 
>   
> geode-pulse/src/test/java/org/apache/geode/tools/pulse/tests/PulseAbstractTest.java
>  5024250f8ff3867e0f527d7ef5fbbd501e2d64ee 
> 
> Diff: https://reviews.apache.org/r/52488/diff/
> 
> 
> Testing
> ---
> 
> precheckin running
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Re: buildSrc prevents geode-core:test

[Jared Stewart]

I don’t know why that happens, but I believe -Dtest.single has been deprecated 
in favor of --tests which lets you specify a test class pattern to match 
against.  The following works for me:
  
./gradlew geode-core:test --tests *QueryDataFunctionApplyLimitClauseTest

(Note that the * is necessary above to avoid having to fully qualify 
org.apache.geode.management.internal.beans.QueryDataFunctionApplyLimitClauseTest.)

Best,
Jared


> On Sep 30, 2016, at 4:03 PM, Kirk Lund  wrote:
> 
> Does anyone know why "buildSrc:test" executes when I try to execute
> "geode-core:test"? See my output below. Instead of executing
> "geode-core:test", gradle executes "buildSrc:test" and fails with "Could
> not find matching test for pattern" -- I think there's something broken in
> our gradle files.
> 
> Thanks,
> Kirk
> 
> /Users/klund/dev/geode [530]$ find . -name
> QueryDataFunctionApplyLimitClauseTest.java
> ./geode-core/src/test/java/org/apache/geode/management/internal/beans/QueryDataFunctionApplyLimitClauseTest.java
> 
> /Users/klund/dev/geode [531]$ ./gradlew
> geode-core:test -Dtest.single=QueryDataFunctionApplyLimitClauseTest
> :buildSrc:compileJava UP-TO-DATE
> :buildSrc:compileGroovy UP-TO-DATE
> :buildSrc:processResources UP-TO-DATE
> :buildSrc:classes UP-TO-DATE
> :buildSrc:jar UP-TO-DATE
> :buildSrc:assemble UP-TO-DATE
> :buildSrc:compileTestJava UP-TO-DATE
> :buildSrc:compileTestGroovy UP-TO-DATE
> :buildSrc:processTestResources UP-TO-DATE
> :buildSrc:testClasses UP-TO-DATE
> :buildSrc:test FAILED
> 
> FAILURE: Build failed with an exception.
> 
> * What went wrong:
> Execution failed for task ':test'.
>> Could not find matching test for pattern:
> QueryDataFunctionApplyLimitClauseTest
> 
> * Try:
> Run with --stacktrace option to get the stack trace. Run with --info or
> --debug option to get more log output.
> 
> BUILD FAILED
> 
> Total time: 0.62 secs
> /Users/klund/dev/geode [532]$

Re: [ANNOUNCE] Donation of Geode documentation

[Jared Stewart]

Thanks for the good news!

On Sep 29, 2016 8:52 PM, "Kirk Lund"  wrote:

> Great news!
>
> On Thursday, September 29, 2016, Anthony Baker  wrote:
>
> > I am pleased to announce the donation of Geode documentation to the
> > Geode community.
> >
> > The documentation provides a complete user guide for Geode. This
> > donation includes the source necessary to build the documentation site
> > currently hosted by Pivotal [1]. The documentation source has been
> > converted into a community-friendly markdown format.
> >
> > The Software Grant Agreement for this code has been accepted by the ASF
> > secretary.
> >
> > The donated source currently sits in a separate branch in the Geode
> > repository named staging/docs-grant1 [2] and is awaiting community
> > review. I encourage everyone in the Geode community to review this
> > donation and provide feedback. Once the community has reached a
> > consensus we can determine next steps and how this code might get
> > merged into the develop branch [3]. This will allow the geode
> > community to accept documentation contributions and self-host the
> > documentation on the project website. Your suggestions are most
> > welcome!
> >
> > Thanks,
> > Anthony
> >
> > [1] http://geode.docs.pivotal.io
> > [2] https://git-wip-us.apache.org/repos/asf?p=incubator-geode.
> > git;a=tree;h=refs/heads/staging/docs-grant1;hb=refs/
> > heads/staging/docs-grant1
> > [3] https://issues.apache.org/jira/browse/GEODE-1952
> >
>

IntelliJ Code Style XML

[Jared Stewart]

From where can I obtain the intellijIdeaCodeStyle.xml file referenced in the 
wiki page  
about code style?

Thanks,
Jared

Re: [GitHub] incubator-geode pull request #240: GEODE-1899: Renamed the custom 'clean' ta...

[Jared Stewart]

Thanks Anthony!  You should just continue to type ‘gradle clean build’ as you 
did before.  

To give more detail:
Each gradle subproject has its own built-in ‘clean’ task, which will remove the 
build directory inside of that module (e.g. geode/geode-core/build/). There is 
also the ‘cleanAll’ task which will remove the build directory from the root 
project (i.e. geode/build/).  The last block of our root build.gradle file 
declares that ‘cleanAll’ must be run after the ‘clean’ task of any subproject, 
so whenever you call ‘gradle clean’ it will automatically trigger ‘cleanAll’.

Best,
Jared 
> On Sep 15, 2016, at 3:42 PM, Anthony Baker <aba...@pivotal.io> wrote:
> 
> Jared, thanks for the contribution!
> 
> Just to make sure everyone understands the change:  I should now type `gradle 
> cleanAll build` instead of `gradle clean build`?
> 
> Anthony
> 
> 
>> On Sep 15, 2016, at 2:44 PM, jaredjstewart <g...@git.apache.org> wrote:
>> 
>> GitHub user jaredjstewart opened a pull request:
>> 
>>   https://github.com/apache/incubator-geode/pull/240
>> 
>>   GEODE-1899: Renamed the custom 'clean' task to work with Gradle 3.0+
>> 
>> 
>> 
>> You can merge this pull request into a Git repository by running:
>> 
>>   $ git pull https://github.com/jaredjstewart/incubator-geode 
>> feature/GEODE-1899
>> 
>> Alternatively you can review and apply these changes as the patch at:
>> 
>>   https://github.com/apache/incubator-geode/pull/240.patch
>> 
>> To close this pull request, make a commit to your master/trunk branch
>> with (at least) the following in the commit message:
>> 
>>   This closes #240
>> 
>> 
>> commit 804a1cabc78357bea13844dd767afcc66a7426af
>> Author: Jared Stewart <jstew...@pivotal.io>
>> Date:   2016-09-15T20:24:34Z
>> 
>>   GEODE-1899: Renamed the custom 'clean' task to work with Gradle 3.0+
>> 
>> 
>> 
>> 
>> ---
>> If your project is set up for it, you can reply to this email and have your
>> reply appear on GitHub as well. If your project does not have this feature
>> enabled and wishes so, or if the feature is enabled but not working, please
>> contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
>> with INFRA.
>> ---
>