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/11/18 04:31:09 UTC
Review Request 40425: GEODE-301 Generate pulseversion.properties file
from gradle build
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/
-----------------------------------------------------------
Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
Repository: geode
Description
-------
pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
Diffs
-----
pulse/.classpath 0052fa2
pulse/.project 7c21b55
pulse/build.bat 366166e
pulse/build.gradle 509ada1
pulse/build.sh 0a9f8d2
pulse/build.xml f7307a1
pulse/buildfiles/dependencies.xml 19c9224
pulse/buildfiles/osplatform.xml cc63ddf
pulse/buildfiles/utilities.xml d01ab42
pulse/src/main/resources/pulseversion.properties d9c777b
pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
Diff: https://reviews.apache.org/r/40425/diff/
Testing
-------
Tested on the local machine - see attached screenshot.
UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
File Attachments
----------------
Pulse Screenshot
https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
Thanks,
Nitin Lamba
Re: Review Request 40425: GEODE-301 Generate pulseversion.properties
file from gradle build
Posted by Nitin Lamba <ni...@ampool.io>.
> On Nov. 20, 2015, 10:24 p.m., Dan Smith wrote:
> > The changes look ok, but it would be easier to review if you update the diff using the update diff button (or the command line rbt), rather than posting a new patch file. That way, reviewboard can show the changes between your first patch and your second patch.
Good point Dan. I too realized it after I published my updated diff. Doing a delta update is much more convenient. Sorry about the inconvenience.
- Nitin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/#review107428
-----------------------------------------------------------
On Nov. 18, 2015, 8:44 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40425/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2015, 8:44 p.m.)
>
>
> Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
>
> Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
>
>
> Diffs
> -----
>
> pulse/.classpath 0052fa2
> pulse/.project 7c21b55
> pulse/build.bat 366166e
> pulse/build.gradle 509ada1
> pulse/build.sh 0a9f8d2
> pulse/build.xml f7307a1
> pulse/buildfiles/dependencies.xml 19c9224
> pulse/buildfiles/osplatform.xml cc63ddf
> pulse/buildfiles/utilities.xml d01ab42
> pulse/src/main/resources/pulseversion.properties d9c777b
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
>
> Diff: https://reviews.apache.org/r/40425/diff/
>
>
> Testing
> -------
>
> Tested on the local machine - see attached screenshot.
>
> UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
>
>
> File Attachments
> ----------------
>
> Pulse Screenshot
> https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
> Updated Diff
> https://reviews.apache.org/media/uploaded/files/2015/11/18/1e81b7e7-249a-48d3-8cbe-de464a85f65f__patch.diff
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 40425: GEODE-301 Generate pulseversion.properties
file from gradle build
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/#review107428
-----------------------------------------------------------
Ship it!
The changes look ok, but it would be easier to review if you update the diff using the update diff button (or the command line rbt), rather than posting a new patch file. That way, reviewboard can show the changes between your first patch and your second patch.
- Dan Smith
On Nov. 18, 2015, 8:44 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40425/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2015, 8:44 p.m.)
>
>
> Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
>
> Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
>
>
> Diffs
> -----
>
> pulse/.classpath 0052fa2
> pulse/.project 7c21b55
> pulse/build.bat 366166e
> pulse/build.gradle 509ada1
> pulse/build.sh 0a9f8d2
> pulse/build.xml f7307a1
> pulse/buildfiles/dependencies.xml 19c9224
> pulse/buildfiles/osplatform.xml cc63ddf
> pulse/buildfiles/utilities.xml d01ab42
> pulse/src/main/resources/pulseversion.properties d9c777b
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
>
> Diff: https://reviews.apache.org/r/40425/diff/
>
>
> Testing
> -------
>
> Tested on the local machine - see attached screenshot.
>
> UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
>
>
> File Attachments
> ----------------
>
> Pulse Screenshot
> https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
> Updated Diff
> https://reviews.apache.org/media/uploaded/files/2015/11/18/1e81b7e7-249a-48d3-8cbe-de464a85f65f__patch.diff
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 40425: GEODE-301 Generate pulseversion.properties
file from gradle build
Posted by Dick Cavender <dc...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/#review107813
-----------------------------------------------------------
Ship it!
Is there any reason the dialog box around the version can't be expanded to a width beyond the revsion? It looks sorta hokey to have the revision overflow the box.
The buildID is still showing a date string while we're still working to move to change this to a commit count instead but that's not going to be fixed here.
- Dick Cavender
On Nov. 18, 2015, 8:44 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40425/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2015, 8:44 p.m.)
>
>
> Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
>
> Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
>
>
> Diffs
> -----
>
> pulse/.classpath 0052fa2
> pulse/.project 7c21b55
> pulse/build.bat 366166e
> pulse/build.gradle 509ada1
> pulse/build.sh 0a9f8d2
> pulse/build.xml f7307a1
> pulse/buildfiles/dependencies.xml 19c9224
> pulse/buildfiles/osplatform.xml cc63ddf
> pulse/buildfiles/utilities.xml d01ab42
> pulse/src/main/resources/pulseversion.properties d9c777b
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
>
> Diff: https://reviews.apache.org/r/40425/diff/
>
>
> Testing
> -------
>
> Tested on the local machine - see attached screenshot.
>
> UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
>
>
> File Attachments
> ----------------
>
> Pulse Screenshot
> https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
> Updated Diff
> https://reviews.apache.org/media/uploaded/files/2015/11/18/1e81b7e7-249a-48d3-8cbe-de464a85f65f__patch.diff
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 40425: GEODE-301 Generate pulseversion.properties
file from gradle build
Posted by Nitin Lamba <ni...@ampool.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/
-----------------------------------------------------------
(Updated Nov. 18, 2015, 8:44 p.m.)
Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
Changes
-------
Re-submitting patch after feedback
Repository: geode
Description
-------
pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
Diffs
-----
pulse/.classpath 0052fa2
pulse/.project 7c21b55
pulse/build.bat 366166e
pulse/build.gradle 509ada1
pulse/build.sh 0a9f8d2
pulse/build.xml f7307a1
pulse/buildfiles/dependencies.xml 19c9224
pulse/buildfiles/osplatform.xml cc63ddf
pulse/buildfiles/utilities.xml d01ab42
pulse/src/main/resources/pulseversion.properties d9c777b
pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
Diff: https://reviews.apache.org/r/40425/diff/
Testing
-------
Tested on the local machine - see attached screenshot.
UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
File Attachments (updated)
----------------
Pulse Screenshot
https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
Updated Diff
https://reviews.apache.org/media/uploaded/files/2015/11/18/1e81b7e7-249a-48d3-8cbe-de464a85f65f__patch.diff
Thanks,
Nitin Lamba
Re: Review Request 40425: GEODE-301 Generate pulseversion.properties
file from gradle build
Posted by Nitin Lamba <ni...@ampool.io>.
On Nov. 18, 2015, 7:50 p.m., Nitin Lamba wrote:
> > Thanks for working on this!
Thanks for the suggestions! I've resolved all of these.
- Nitin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/#review107072
-----------------------------------------------------------
On Nov. 18, 2015, 8:44 p.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40425/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2015, 8:44 p.m.)
>
>
> Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
>
> Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
>
>
> Diffs
> -----
>
> pulse/.classpath 0052fa2
> pulse/.project 7c21b55
> pulse/build.bat 366166e
> pulse/build.gradle 509ada1
> pulse/build.sh 0a9f8d2
> pulse/build.xml f7307a1
> pulse/buildfiles/dependencies.xml 19c9224
> pulse/buildfiles/osplatform.xml cc63ddf
> pulse/buildfiles/utilities.xml d01ab42
> pulse/src/main/resources/pulseversion.properties d9c777b
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
>
> Diff: https://reviews.apache.org/r/40425/diff/
>
>
> Testing
> -------
>
> Tested on the local machine - see attached screenshot.
>
> UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
>
>
> File Attachments
> ----------------
>
> Pulse Screenshot
> https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
> Updated Diff
> https://reviews.apache.org/media/uploaded/files/2015/11/18/1e81b7e7-249a-48d3-8cbe-de464a85f65f__patch.diff
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 40425: GEODE-301 Generate pulseversion.properties
file from gradle build
Posted by Anthony Baker <ab...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/#review107072
-----------------------------------------------------------
pulse/build.gradle (line 15)
<https://reviews.apache.org/r/40425/#comment165998>
I think this should be $buildDir/generated-resources/main
pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java (line 31)
<https://reviews.apache.org/r/40425/#comment165993>
Suggestion: delete dead code.
pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java (line 35)
<https://reviews.apache.org/r/40425/#comment165994>
Suggestion: use try-with-resources to ensure input stream is not leaked.
Thanks for working on this!
- Anthony Baker
On Nov. 18, 2015, 3:31 a.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40425/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2015, 3:31 a.m.)
>
>
> Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
>
> Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
>
>
> Diffs
> -----
>
> pulse/.classpath 0052fa2
> pulse/.project 7c21b55
> pulse/build.bat 366166e
> pulse/build.gradle 509ada1
> pulse/build.sh 0a9f8d2
> pulse/build.xml f7307a1
> pulse/buildfiles/dependencies.xml 19c9224
> pulse/buildfiles/osplatform.xml cc63ddf
> pulse/buildfiles/utilities.xml d01ab42
> pulse/src/main/resources/pulseversion.properties d9c777b
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
>
> Diff: https://reviews.apache.org/r/40425/diff/
>
>
> Testing
> -------
>
> Tested on the local machine - see attached screenshot.
>
> UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
>
>
> File Attachments
> ----------------
>
> Pulse Screenshot
> https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
>
>
> Thanks,
>
> Nitin Lamba
>
>
Re: Review Request 40425: GEODE-301 Generate pulseversion.properties
file from gradle build
Posted by Dan Smith <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40425/#review107068
-----------------------------------------------------------
Ship it!
Ship It!
- Dan Smith
On Nov. 18, 2015, 3:31 a.m., Nitin Lamba wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40425/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2015, 3:31 a.m.)
>
>
> Review request for geode, Anthony Baker, Dick Cavender, Mark Bretl, and Dan Smith.
>
>
> Repository: geode
>
>
> Description
> -------
>
> pulseversion.properties file is now generated from pulse gradle build file. Other older (ant) build files are removed from the repo and few pending test file cleanups.
>
> Most of the script is duplicated from gemfire-core module (thanks Mark!). Once this feature is merged into develop, we can further optimize by moving this task to the top-level project.
>
>
> Diffs
> -----
>
> pulse/.classpath 0052fa2
> pulse/.project 7c21b55
> pulse/build.bat 366166e
> pulse/build.gradle 509ada1
> pulse/build.sh 0a9f8d2
> pulse/build.xml f7307a1
> pulse/buildfiles/dependencies.xml 19c9224
> pulse/buildfiles/osplatform.xml cc63ddf
> pulse/buildfiles/utilities.xml d01ab42
> pulse/src/main/resources/pulseversion.properties d9c777b
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/testbed/TestBed.java b68e0da
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/PulseTests.java 3401394
> pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/junit/BaseServiceTest.java e98cdda
>
> Diff: https://reviews.apache.org/r/40425/diff/
>
>
> Testing
> -------
>
> Tested on the local machine - see attached screenshot.
>
> UI layout needs a bit of fixing, which will be picked-up with GEODE-511 (Fixing license headers and branding).
>
>
> File Attachments
> ----------------
>
> Pulse Screenshot
> https://reviews.apache.org/media/uploaded/files/2015/11/18/592e89d0-0c7e-4983-874c-cf52c6d4f598__Screen_Shot_2015-11-17_at_7.04.50_PM.png
>
>
> Thanks,
>
> Nitin Lamba
>
>