You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Nitin Lamba <ni...@ampool.io> on 2015/09/02 18:32:53 UTC
Review Request 38060: GEODE-300 new gradle script for Pulse
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/
-----------------------------------------------------------
Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
Repository: geode
Description
-------
Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
These chances will be tracked on GEODE-12 branch until all subtasks are completed.
Diffs
-----
build.gradle 3bad39c
gemfire-assembly/build.gradle 0e51563
gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
pulse/build.gradle PRE-CREATION
settings.gradle 7f6ed61
Diff: https://reviews.apache.org/r/38060/diff/
Testing
-------
- pulse.war file gets generated correctly and placed in tools/Pulse folder
- Starting locator starts pulse within the embedded jetty instance
- Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
Thanks,
Nitin Lamba
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/#review98072
-----------------------------------------------------------
Ship it!
Ship It!
- Dan Smith
On Sept. 4, 2015, 6:51 a.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2015, 6:51 a.m.)
>
>
> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>
> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>
>
> Diffs
> -----
>
> build.gradle 3bad39c
> gemfire-assembly/build.gradle 0e51563
> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> pulse/build.gradle PRE-CREATION
> settings.gradle 7f6ed61
>
> Diff: https://reviews.apache.org/r/38060/diff/
>
>
> Testing
> -------
>
> - pulse.war file gets generated correctly and placed in tools/Pulse folder
> - Starting locator starts pulse within the embedded jetty instance
> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Jens Deppe <je...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/#review98070
-----------------------------------------------------------
Ship it!
Ship It!
- Jens Deppe
On Sept. 4, 2015, 6:51 a.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2015, 6:51 a.m.)
>
>
> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>
> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>
>
> Diffs
> -----
>
> build.gradle 3bad39c
> gemfire-assembly/build.gradle 0e51563
> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> pulse/build.gradle PRE-CREATION
> settings.gradle 7f6ed61
>
> Diff: https://reviews.apache.org/r/38060/diff/
>
>
> Testing
> -------
>
> - pulse.war file gets generated correctly and placed in tools/Pulse folder
> - Starting locator starts pulse within the embedded jetty instance
> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Nitin Lamba <ni...@ampool.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/
-----------------------------------------------------------
(Updated Sept. 4, 2015, 6:51 a.m.)
Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
Changes
-------
Remove custom configurations and updating Spring Framework dependencies to 3.2.12
Repository: geode
Description
-------
Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
These chances will be tracked on GEODE-12 branch until all subtasks are completed.
Diffs (updated)
-----
build.gradle 3bad39c
gemfire-assembly/build.gradle 0e51563
gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
pulse/build.gradle PRE-CREATION
settings.gradle 7f6ed61
Diff: https://reviews.apache.org/r/38060/diff/
Testing
-------
- pulse.war file gets generated correctly and placed in tools/Pulse folder
- Starting locator starts pulse within the embedded jetty instance
- Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
Thanks,
Nitin Lamba
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Mark Bretl <mb...@pivotal.io>.
> On Sept. 2, 2015, 6:59 p.m., Mark Bretl wrote:
> >
Echoing what Dan said, great job Nitin! Only a few minor points to follow up on.
Thanks for 'porting' the build script from Ant to Gradle!
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/#review97513
-----------------------------------------------------------
On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 6:42 p.m.)
>
>
> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>
> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>
>
> Diffs
> -----
>
> build.gradle 3bad39c
> gemfire-assembly/build.gradle 0e51563
> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> pulse/build.gradle PRE-CREATION
> settings.gradle 7f6ed61
>
> Diff: https://reviews.apache.org/r/38060/diff/
>
>
> Testing
> -------
>
> - pulse.war file gets generated correctly and placed in tools/Pulse folder
> - Starting locator starts pulse within the embedded jetty instance
> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Nitin Lamba <ni...@ampool.io>.
> On Sept. 2, 2015, 6:59 p.m., Mark Bretl wrote:
> > pulse/build.gradle, line 4
> > <https://reviews.apache.org/r/38060/diff/2/?file=1062427#file1062427line4>
> >
> > Why add a custom pulseCompile configuration?
compile target inherits from parent gradle project and use newer versions of Spring framework jars. I tried hard but couldn't override those dependencies from within a sub-project. Besides creating new tasks, the other option is to remove those from parent and move them downstream to different sub-projects using it. Is there a different way?
> On Sept. 2, 2015, 6:59 p.m., Mark Bretl wrote:
> > pulse/build.gradle, line 8
> > <https://reviews.apache.org/r/38060/diff/2/?file=1062427#file1062427line8>
> >
> > Why add a custom testPulseCompile configuration?
testPulseCompile config inherits from parent gradle project and use newer versions of Spring framework jars. I tried hard but couldn't override those dependencies from within a sub-project. Besides creating new tasks, the other option is to remove those from parent and move them downstream to different sub-projects using it. Is there a different way?
> On Sept. 2, 2015, 6:59 p.m., Mark Bretl wrote:
> > pulse/build.gradle, line 81
> > <https://reviews.apache.org/r/38060/diff/2/?file=1062427#file1062427line81>
> >
> > Add war file to archive list?
Yes, Dan raised it too. Have removed that from the archive file.
> On Sept. 2, 2015, 6:59 p.m., Mark Bretl wrote:
> > pulse/build.gradle, line 93
> > <https://reviews.apache.org/r/38060/diff/2/?file=1062427#file1062427line93>
> >
> > This can be removed, only for commercial license
Thanks, it is commented out right now. Will pick-up the actual edits with GEODE-301.
- Nitin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/#review97513
-----------------------------------------------------------
On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 6:42 p.m.)
>
>
> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>
> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>
>
> Diffs
> -----
>
> build.gradle 3bad39c
> gemfire-assembly/build.gradle 0e51563
> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> pulse/build.gradle PRE-CREATION
> settings.gradle 7f6ed61
>
> Diff: https://reviews.apache.org/r/38060/diff/
>
>
> Testing
> -------
>
> - pulse.war file gets generated correctly and placed in tools/Pulse folder
> - Starting locator starts pulse within the embedded jetty instance
> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Mark Bretl <mb...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/#review97513
-----------------------------------------------------------
pulse/build.gradle (line 4)
<https://reviews.apache.org/r/38060/#comment153430>
Why add a custom pulseCompile configuration?
pulse/build.gradle (line 8)
<https://reviews.apache.org/r/38060/#comment153431>
Why add a custom testPulseCompile configuration?
pulse/build.gradle (line 81)
<https://reviews.apache.org/r/38060/#comment153432>
Add war file to archive list?
pulse/build.gradle (line 93)
<https://reviews.apache.org/r/38060/#comment153434>
This can be removed, only for commercial license
- Mark Bretl
On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 6:42 p.m.)
>
>
> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>
> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>
>
> Diffs
> -----
>
> build.gradle 3bad39c
> gemfire-assembly/build.gradle 0e51563
> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> pulse/build.gradle PRE-CREATION
> settings.gradle 7f6ed61
>
> Diff: https://reviews.apache.org/r/38060/diff/
>
>
> Testing
> -------
>
> - pulse.war file gets generated correctly and placed in tools/Pulse folder
> - Starting locator starts pulse within the embedded jetty instance
> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Nitin Lamba <ni...@ampool.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/
-----------------------------------------------------------
(Updated Sept. 2, 2015, 6:42 p.m.)
Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
Repository: geode
Description
-------
Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
These chances will be tracked on GEODE-12 branch until all subtasks are completed.
Diffs (updated)
-----
build.gradle 3bad39c
gemfire-assembly/build.gradle 0e51563
gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
pulse/build.gradle PRE-CREATION
settings.gradle 7f6ed61
Diff: https://reviews.apache.org/r/38060/diff/
Testing
-------
- pulse.war file gets generated correctly and placed in tools/Pulse folder
- Starting locator starts pulse within the embedded jetty instance
- Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
Thanks,
Nitin Lamba
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Nitin Lamba <ni...@ampool.io>.
Following-up on this thread from last week - I had re-submitted the request with updated diff that uses global settings/version for Spring Framework.
Let me know if anything else is pending on this.
Thanks,
Nitin
________________________________________
From: Nitin Lamba <ni...@ampool.io>
Sent: Wednesday, September 2, 2015 4:00 PM
To: Mark Bretl; Anthony Baker
Cc: dev@geode.incubator.apache.org; Dan Smith; Jens Deppe
Subject: Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Great! Makes sense.
I did a quick check and seems that we can safely move Pulse dependencies to 3.2.12, the current Spring release defined in root gradle script. Also, Spring Security modules can be bumped-up to 3.1.7 and Spring LDAP to 1.3.2.
Given how global dependencies are defined in the root project, I'm pulling an additional jar file into war's WEB-INF/lib folder: spring-context-support-3.2.12.RELEASE.jar. I could explicitly exclude it but a cleaner fix is to move that dependency down to sub-project(s) that use it. If we do nothing, the pulse war can carry the extra 0.1 Mb baggage for now :)
Let me know what is preferred - will make changes accordingly.
Thanks,
Nitin
________________________________
From: Mark Bretl <mb...@pivotal.io>
Sent: Wednesday, September 2, 2015 2:10 PM
To: Anthony Baker
Cc: dev@geode.incubator.apache.org; Nitin Lamba; Dan Smith; Jens Deppe
Subject: Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Yes, it does make sense and should be a goal. I think they only got out of sync due to separate teams managing dependencies. Now would be a good time to re-sync them.
--Mark
On Wed, Sep 2, 2015 at 2:04 PM, Anthony Baker <ab...@pivotal.io>> wrote:
Would it make sense to use a single version of Spring jars throughout the build?
> On Sep 2, 2015, at 1:10 PM, Nitin Lamba <ni...@ampool.io>> wrote:
>
>
>
>> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
>>> Hi Nitin,
>>>
>>> Looks like good progress! I had a few comments/questions:
>>>
>>> 1. Is there are reason why you added the pulseCompile, pulseTestCompile configurations rather than just use the standard compile and testCompile ones?
>>> 2. I think it would be better to just rename and categorize the the tests to follow the conventions, rather than add different test and integrationTest targets. Maybe we can add another JIRA for that?
>
> Thanks Dan,
>
> 1. compile and test targets inherit from parent gradle project and use newer versions of Spring framework jars. I tried hard but couldn't override those dependencies from within a sub-project. Besides creating new tasks, the other option is to remove those from parent and move them downstream to different sub-projects using it. Is there a different way?
>
> 2. The new targets are required anyway to add a couple of new system properties. Agree that the tests need to be cleaned-up to follow conventions so I'll update GEODE-304 with those comments
>
>
>> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
>>> gemfire-assembly/build.gradle, line 188
>>> <https://reviews.apache.org/r/38060/diff/1/?file=1062329#file1062329line188>
>>>
>>> I don't think this is necessary to include the pulse jars in the lib directory - at least when I tried dropping a just the pulse war into the tools/Pulse directory with the old gemfire pulse it worked fine. I'm not sure what the classpath implications of putting them one place vs. the other are though.
>
> Thanks. I've removed the lines from gemfire-assembly's build gradle file.
>
>
> - Nitin
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/#review97495
> -----------------------------------------------------------
>
>
> On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/38060/
>> -----------------------------------------------------------
>>
>> (Updated Sept. 2, 2015, 6:42 p.m.)
>>
>>
>> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>>
>>
>> Repository: geode
>>
>>
>> Description
>> -------
>>
>> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>>
>> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>>
>>
>> Diffs
>> -----
>>
>> build.gradle 3bad39c
>> gemfire-assembly/build.gradle 0e51563
>> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
>> pulse/build.gradle PRE-CREATION
>> settings.gradle 7f6ed61
>>
>> Diff: https://reviews.apache.org/r/38060/diff/
>>
>>
>> Testing
>> -------
>>
>> - pulse.war file gets generated correctly and placed in tools/Pulse folder
>> - Starting locator starts pulse within the embedded jetty instance
>> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>>
>>
>> Thanks,
>>
>> Nitin Lamba
>>
>>
>
--
Mark Bretl
Software Build Engineer
Pivotal
503-533-3869
www.pivotal.io<http://www.pivotal.io>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Nitin Lamba <ni...@ampool.io>.
Great! Makes sense.
I did a quick check and seems that we can safely move Pulse dependencies to 3.2.12, the current Spring release defined in root gradle script. Also, Spring Security modules can be bumped-up to 3.1.7 and Spring LDAP to 1.3.2.
Given how global dependencies are defined in the root project, I'm pulling an additional jar file into war's WEB-INF/lib folder: spring-context-support-3.2.12.RELEASE.jar. I could explicitly exclude it but a cleaner fix is to move that dependency down to sub-project(s) that use it. If we do nothing, the pulse war can carry the extra 0.1 Mb baggage for now :)
Let me know what is preferred - will make changes accordingly.
Thanks,
Nitin
________________________________
From: Mark Bretl <mb...@pivotal.io>
Sent: Wednesday, September 2, 2015 2:10 PM
To: Anthony Baker
Cc: dev@geode.incubator.apache.org; Nitin Lamba; Dan Smith; Jens Deppe
Subject: Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Yes, it does make sense and should be a goal. I think they only got out of sync due to separate teams managing dependencies. Now would be a good time to re-sync them.
--Mark
On Wed, Sep 2, 2015 at 2:04 PM, Anthony Baker <ab...@pivotal.io>> wrote:
Would it make sense to use a single version of Spring jars throughout the build?
> On Sep 2, 2015, at 1:10 PM, Nitin Lamba <ni...@ampool.io>> wrote:
>
>
>
>> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
>>> Hi Nitin,
>>>
>>> Looks like good progress! I had a few comments/questions:
>>>
>>> 1. Is there are reason why you added the pulseCompile, pulseTestCompile configurations rather than just use the standard compile and testCompile ones?
>>> 2. I think it would be better to just rename and categorize the the tests to follow the conventions, rather than add different test and integrationTest targets. Maybe we can add another JIRA for that?
>
> Thanks Dan,
>
> 1. compile and test targets inherit from parent gradle project and use newer versions of Spring framework jars. I tried hard but couldn't override those dependencies from within a sub-project. Besides creating new tasks, the other option is to remove those from parent and move them downstream to different sub-projects using it. Is there a different way?
>
> 2. The new targets are required anyway to add a couple of new system properties. Agree that the tests need to be cleaned-up to follow conventions so I'll update GEODE-304 with those comments
>
>
>> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
>>> gemfire-assembly/build.gradle, line 188
>>> <https://reviews.apache.org/r/38060/diff/1/?file=1062329#file1062329line188>
>>>
>>> I don't think this is necessary to include the pulse jars in the lib directory - at least when I tried dropping a just the pulse war into the tools/Pulse directory with the old gemfire pulse it worked fine. I'm not sure what the classpath implications of putting them one place vs. the other are though.
>
> Thanks. I've removed the lines from gemfire-assembly's build gradle file.
>
>
> - Nitin
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/#review97495
> -----------------------------------------------------------
>
>
> On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/38060/
>> -----------------------------------------------------------
>>
>> (Updated Sept. 2, 2015, 6:42 p.m.)
>>
>>
>> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>>
>>
>> Repository: geode
>>
>>
>> Description
>> -------
>>
>> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>>
>> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>>
>>
>> Diffs
>> -----
>>
>> build.gradle 3bad39c
>> gemfire-assembly/build.gradle 0e51563
>> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
>> pulse/build.gradle PRE-CREATION
>> settings.gradle 7f6ed61
>>
>> Diff: https://reviews.apache.org/r/38060/diff/
>>
>>
>> Testing
>> -------
>>
>> - pulse.war file gets generated correctly and placed in tools/Pulse folder
>> - Starting locator starts pulse within the embedded jetty instance
>> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>>
>>
>> Thanks,
>>
>> Nitin Lamba
>>
>>
>
--
Mark Bretl
Software Build Engineer
Pivotal
503-533-3869
www.pivotal.io<http://www.pivotal.io>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Mark Bretl <mb...@pivotal.io>.
Yes, it does make sense and should be a goal. I think they only got out of
sync due to separate teams managing dependencies. Now would be a good time
to re-sync them.
--Mark
On Wed, Sep 2, 2015 at 2:04 PM, Anthony Baker <ab...@pivotal.io> wrote:
> Would it make sense to use a single version of Spring jars throughout the
> build?
>
> > On Sep 2, 2015, at 1:10 PM, Nitin Lamba <ni...@ampool.io> wrote:
> >
> >
> >
> >> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
> >>> Hi Nitin,
> >>>
> >>> Looks like good progress! I had a few comments/questions:
> >>>
> >>> 1. Is there are reason why you added the pulseCompile,
> pulseTestCompile configurations rather than just use the standard compile
> and testCompile ones?
> >>> 2. I think it would be better to just rename and categorize the the
> tests to follow the conventions, rather than add different test and
> integrationTest targets. Maybe we can add another JIRA for that?
> >
> > Thanks Dan,
> >
> > 1. compile and test targets inherit from parent gradle project and use
> newer versions of Spring framework jars. I tried hard but couldn't override
> those dependencies from within a sub-project. Besides creating new tasks,
> the other option is to remove those from parent and move them downstream to
> different sub-projects using it. Is there a different way?
> >
> > 2. The new targets are required anyway to add a couple of new system
> properties. Agree that the tests need to be cleaned-up to follow
> conventions so I'll update GEODE-304 with those comments
> >
> >
> >> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
> >>> gemfire-assembly/build.gradle, line 188
> >>> <
> https://reviews.apache.org/r/38060/diff/1/?file=1062329#file1062329line188
> >
> >>>
> >>> I don't think this is necessary to include the pulse jars in the
> lib directory - at least when I tried dropping a just the pulse war into
> the tools/Pulse directory with the old gemfire pulse it worked fine. I'm
> not sure what the classpath implications of putting them one place vs. the
> other are though.
> >
> > Thanks. I've removed the lines from gemfire-assembly's build gradle file.
> >
> >
> > - Nitin
> >
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/38060/#review97495
> > -----------------------------------------------------------
> >
> >
> > On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
> >>
> >> -----------------------------------------------------------
> >> This is an automatically generated e-mail. To reply, visit:
> >> https://reviews.apache.org/r/38060/
> >> -----------------------------------------------------------
> >>
> >> (Updated Sept. 2, 2015, 6:42 p.m.)
> >>
> >>
> >> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
> >>
> >>
> >> Repository: geode
> >>
> >>
> >> Description
> >> -------
> >>
> >> Create new gradle script for PULSE to build archives (jar, war files)
> using the dependencies from existing ant build.xml file.
> >>
> >> These chances will be tracked on GEODE-12 branch until all subtasks are
> completed.
> >>
> >>
> >> Diffs
> >> -----
> >>
> >> build.gradle 3bad39c
> >> gemfire-assembly/build.gradle 0e51563
> >> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> >> pulse/build.gradle PRE-CREATION
> >> settings.gradle 7f6ed61
> >>
> >> Diff: https://reviews.apache.org/r/38060/diff/
> >>
> >>
> >> Testing
> >> -------
> >>
> >> - pulse.war file gets generated correctly and placed in tools/Pulse
> folder
> >> - Starting locator starts pulse within the embedded jetty instance
> >> - Unit and integration tests are blocked. GEODE-304 is tracking the
> issue separately
> >>
> >>
> >> Thanks,
> >>
> >> Nitin Lamba
> >>
> >>
> >
>
>
--
Mark Bretl
Software Build Engineer
Pivotal
503-533-3869
www.pivotal.io
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Anthony Baker <ab...@pivotal.io>.
Would it make sense to use a single version of Spring jars throughout the build?
> On Sep 2, 2015, at 1:10 PM, Nitin Lamba <ni...@ampool.io> wrote:
>
>
>
>> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
>>> Hi Nitin,
>>>
>>> Looks like good progress! I had a few comments/questions:
>>>
>>> 1. Is there are reason why you added the pulseCompile, pulseTestCompile configurations rather than just use the standard compile and testCompile ones?
>>> 2. I think it would be better to just rename and categorize the the tests to follow the conventions, rather than add different test and integrationTest targets. Maybe we can add another JIRA for that?
>
> Thanks Dan,
>
> 1. compile and test targets inherit from parent gradle project and use newer versions of Spring framework jars. I tried hard but couldn't override those dependencies from within a sub-project. Besides creating new tasks, the other option is to remove those from parent and move them downstream to different sub-projects using it. Is there a different way?
>
> 2. The new targets are required anyway to add a couple of new system properties. Agree that the tests need to be cleaned-up to follow conventions so I'll update GEODE-304 with those comments
>
>
>> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
>>> gemfire-assembly/build.gradle, line 188
>>> <https://reviews.apache.org/r/38060/diff/1/?file=1062329#file1062329line188>
>>>
>>> I don't think this is necessary to include the pulse jars in the lib directory - at least when I tried dropping a just the pulse war into the tools/Pulse directory with the old gemfire pulse it worked fine. I'm not sure what the classpath implications of putting them one place vs. the other are though.
>
> Thanks. I've removed the lines from gemfire-assembly's build gradle file.
>
>
> - Nitin
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/#review97495
> -----------------------------------------------------------
>
>
> On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/38060/
>> -----------------------------------------------------------
>>
>> (Updated Sept. 2, 2015, 6:42 p.m.)
>>
>>
>> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>>
>>
>> Repository: geode
>>
>>
>> Description
>> -------
>>
>> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>>
>> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>>
>>
>> Diffs
>> -----
>>
>> build.gradle 3bad39c
>> gemfire-assembly/build.gradle 0e51563
>> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
>> pulse/build.gradle PRE-CREATION
>> settings.gradle 7f6ed61
>>
>> Diff: https://reviews.apache.org/r/38060/diff/
>>
>>
>> Testing
>> -------
>>
>> - pulse.war file gets generated correctly and placed in tools/Pulse folder
>> - Starting locator starts pulse within the embedded jetty instance
>> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>>
>>
>> Thanks,
>>
>> Nitin Lamba
>>
>>
>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Nitin Lamba <ni...@ampool.io>.
> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
> > Hi Nitin,
> >
> > Looks like good progress! I had a few comments/questions:
> >
> > 1. Is there are reason why you added the pulseCompile, pulseTestCompile configurations rather than just use the standard compile and testCompile ones?
> > 2. I think it would be better to just rename and categorize the the tests to follow the conventions, rather than add different test and integrationTest targets. Maybe we can add another JIRA for that?
Thanks Dan,
1. compile and test targets inherit from parent gradle project and use newer versions of Spring framework jars. I tried hard but couldn't override those dependencies from within a sub-project. Besides creating new tasks, the other option is to remove those from parent and move them downstream to different sub-projects using it. Is there a different way?
2. The new targets are required anyway to add a couple of new system properties. Agree that the tests need to be cleaned-up to follow conventions so I'll update GEODE-304 with those comments
> On Sept. 2, 2015, 5:35 p.m., Dan Smith wrote:
> > gemfire-assembly/build.gradle, line 188
> > <https://reviews.apache.org/r/38060/diff/1/?file=1062329#file1062329line188>
> >
> > I don't think this is necessary to include the pulse jars in the lib directory - at least when I tried dropping a just the pulse war into the tools/Pulse directory with the old gemfire pulse it worked fine. I'm not sure what the classpath implications of putting them one place vs. the other are though.
Thanks. I've removed the lines from gemfire-assembly's build gradle file.
- Nitin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/#review97495
-----------------------------------------------------------
On Sept. 2, 2015, 6:42 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 6:42 p.m.)
>
>
> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>
> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>
>
> Diffs
> -----
>
> build.gradle 3bad39c
> gemfire-assembly/build.gradle 0e51563
> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> pulse/build.gradle PRE-CREATION
> settings.gradle 7f6ed61
>
> Diff: https://reviews.apache.org/r/38060/diff/
>
>
> Testing
> -------
>
> - pulse.war file gets generated correctly and placed in tools/Pulse folder
> - Starting locator starts pulse within the embedded jetty instance
> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 38060: GEODE-300 new gradle script for Pulse
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38060/#review97495
-----------------------------------------------------------
Ship it!
Hi Nitin,
Looks like good progress! I had a few comments/questions:
1. Is there are reason why you added the pulseCompile, pulseTestCompile configurations rather than just use the standard compile and testCompile ones?
2. I think it would be better to just rename and categorize the the tests to follow the conventions, rather than add different test and integrationTest targets. Maybe we can add another JIRA for that?
gemfire-assembly/build.gradle (line 188)
<https://reviews.apache.org/r/38060/#comment153387>
I don't think this is necessary to include the pulse jars in the lib directory - at least when I tried dropping a just the pulse war into the tools/Pulse directory with the old gemfire pulse it worked fine. I'm not sure what the classpath implications of putting them one place vs. the other are though.
- Dan Smith
On Sept. 2, 2015, 4:32 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38060/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2015, 4:32 p.m.)
>
>
> Review request for geode, Jens Deppe, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Create new gradle script for PULSE to build archives (jar, war files) using the dependencies from existing ant build.xml file.
>
> These chances will be tracked on GEODE-12 branch until all subtasks are completed.
>
>
> Diffs
> -----
>
> build.gradle 3bad39c
> gemfire-assembly/build.gradle 0e51563
> gemfire-assembly/src/test/java/AgentUtilJUnitTest.java 0f7563b
> pulse/build.gradle PRE-CREATION
> settings.gradle 7f6ed61
>
> Diff: https://reviews.apache.org/r/38060/diff/
>
>
> Testing
> -------
>
> - pulse.war file gets generated correctly and placed in tools/Pulse folder
> - Starting locator starts pulse within the embedded jetty instance
> - Unit and integration tests are blocked. GEODE-304 is tracking the issue separately
>
>
> Thanks,
>
> Nitin Lamba
>
>