Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Anthony Baker


> On July 26, 2016, 5:02 p.m., Dan Smith wrote:
> > Looks good - but this does mean that the build id will always be 0 unless 
> > someone sets it, whereas before it was the current date. Will that be an 
> > issue?
> 
> Anthony Baker wrote:
> AFAIK, the buildId is only used in GemFireVersion.properties.  That file 
> already has a more useful build date embedded.
> 
> Mark Bretl wrote:
> The BuildId is used when getting the 'full' version command in GFSH, 
> 'gfsh gfsh version --full'
> 
> Example:
> 
> $ ./gfsh version --full
> Java version:   1.0.0-incubating-SNAPSHOT mbretl 072616 2016-07-26 
> 10:59:05 -0700 javac 1.8.0_65
> Native version: native code unavailable
> Source revision: b3b2c89098efb71d57c7788b03cf6c933bdd58b1
> Source repository: develop
> Running on: /172.16.120.55, 8 cpu(s), x86_64 Mac OS X 10.11.5
> 
> 
> Before the BuildId was a date reference, it was the SVN revision. Once 
> the source was moved to Git, we had to change it. It does hold some value in 
> keeping the concept of BuildId, so individual builds can be distinguished 
> from running the gfsh command. However, as you can see from the gfsh output, 
> the current 'date' value from the BuildId (072616) is redundant to the 'real' 
> date next to it. I don't feel like setting it to '0' accomplishes much 
> without purpose behind it.
> 
> One option could be to use 'git rev-list HEAD --count' to get a more 
> unique value. I don't it was added initially since most repositories for 
> RHEL/CentOS 5 didn't have a version of Git with the rev-list command, which I 
> think came with Git version 1.8+.
> 
> Anthony Baker wrote:
> Thanks Mark!  IMHO I would rather rely on the git sha instead of a commit 
> count.  Adding the buildId to the gradle properties makes it easy to override 
> on the cmd line:  `gradle -PbuildId=42 build`.  I see it being useful as 
> semver "build metadata".
> 
> Mark Bretl wrote:
> Agree, I would not rely on the count since it could be different since 
> anyone with their own repository could have a different revision count with 
> Git.
> 
> Being able to overwrite it is a good feature. Would it be worth adding a 
> comment above in the gradle.properties to explain the use and possibly why to 
> change? Otherwise, I think it will simply be 0 and people wouldn't know what 
> it means.

Will do!


- Anthony


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


On July 26, 2016, 4:56 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50436/
> ---
> 
> (Updated July 26, 2016, 4:56 p.m.)
> 
> 
> Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Extract the product name and org into gradle.properties.
> 
> GEODE-1695: Add buildId to gradle.properties
> 
> 
> GEODE-1695: Use productName to build javadocs
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-assembly/build.gradle 
> 412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
>   geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
>   geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
>   gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
>   gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 
> 
> Diff: https://reviews.apache.org/r/50436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>



Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Mark Bretl


> On July 26, 2016, 10:02 a.m., Dan Smith wrote:
> > Looks good - but this does mean that the build id will always be 0 unless 
> > someone sets it, whereas before it was the current date. Will that be an 
> > issue?
> 
> Anthony Baker wrote:
> AFAIK, the buildId is only used in GemFireVersion.properties.  That file 
> already has a more useful build date embedded.
> 
> Mark Bretl wrote:
> The BuildId is used when getting the 'full' version command in GFSH, 
> 'gfsh gfsh version --full'
> 
> Example:
> 
> $ ./gfsh version --full
> Java version:   1.0.0-incubating-SNAPSHOT mbretl 072616 2016-07-26 
> 10:59:05 -0700 javac 1.8.0_65
> Native version: native code unavailable
> Source revision: b3b2c89098efb71d57c7788b03cf6c933bdd58b1
> Source repository: develop
> Running on: /172.16.120.55, 8 cpu(s), x86_64 Mac OS X 10.11.5
> 
> 
> Before the BuildId was a date reference, it was the SVN revision. Once 
> the source was moved to Git, we had to change it. It does hold some value in 
> keeping the concept of BuildId, so individual builds can be distinguished 
> from running the gfsh command. However, as you can see from the gfsh output, 
> the current 'date' value from the BuildId (072616) is redundant to the 'real' 
> date next to it. I don't feel like setting it to '0' accomplishes much 
> without purpose behind it.
> 
> One option could be to use 'git rev-list HEAD --count' to get a more 
> unique value. I don't it was added initially since most repositories for 
> RHEL/CentOS 5 didn't have a version of Git with the rev-list command, which I 
> think came with Git version 1.8+.
> 
> Anthony Baker wrote:
> Thanks Mark!  IMHO I would rather rely on the git sha instead of a commit 
> count.  Adding the buildId to the gradle properties makes it easy to override 
> on the cmd line:  `gradle -PbuildId=42 build`.  I see it being useful as 
> semver "build metadata".

Agree, I would not rely on the count since it could be different since anyone 
with their own repository could have a different revision count with Git.

Being able to overwrite it is a good feature. Would it be worth adding a 
comment above in the gradle.properties to explain the use and possibly why to 
change? Otherwise, I think it will simply be 0 and people wouldn't know what it 
means.


- Mark


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


On July 26, 2016, 9:56 a.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50436/
> ---
> 
> (Updated July 26, 2016, 9:56 a.m.)
> 
> 
> Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Extract the product name and org into gradle.properties.
> 
> GEODE-1695: Add buildId to gradle.properties
> 
> 
> GEODE-1695: Use productName to build javadocs
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-assembly/build.gradle 
> 412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
>   geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
>   geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
>   gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
>   gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 
> 
> Diff: https://reviews.apache.org/r/50436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>



Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Anthony Baker


> On July 26, 2016, 5:02 p.m., Dan Smith wrote:
> > Looks good - but this does mean that the build id will always be 0 unless 
> > someone sets it, whereas before it was the current date. Will that be an 
> > issue?
> 
> Anthony Baker wrote:
> AFAIK, the buildId is only used in GemFireVersion.properties.  That file 
> already has a more useful build date embedded.
> 
> Mark Bretl wrote:
> The BuildId is used when getting the 'full' version command in GFSH, 
> 'gfsh gfsh version --full'
> 
> Example:
> 
> $ ./gfsh version --full
> Java version:   1.0.0-incubating-SNAPSHOT mbretl 072616 2016-07-26 
> 10:59:05 -0700 javac 1.8.0_65
> Native version: native code unavailable
> Source revision: b3b2c89098efb71d57c7788b03cf6c933bdd58b1
> Source repository: develop
> Running on: /172.16.120.55, 8 cpu(s), x86_64 Mac OS X 10.11.5
> 
> 
> Before the BuildId was a date reference, it was the SVN revision. Once 
> the source was moved to Git, we had to change it. It does hold some value in 
> keeping the concept of BuildId, so individual builds can be distinguished 
> from running the gfsh command. However, as you can see from the gfsh output, 
> the current 'date' value from the BuildId (072616) is redundant to the 'real' 
> date next to it. I don't feel like setting it to '0' accomplishes much 
> without purpose behind it.
> 
> One option could be to use 'git rev-list HEAD --count' to get a more 
> unique value. I don't it was added initially since most repositories for 
> RHEL/CentOS 5 didn't have a version of Git with the rev-list command, which I 
> think came with Git version 1.8+.

Thanks Mark!  IMHO I would rather rely on the git sha instead of a commit 
count.  Adding the buildId to the gradle properties makes it easy to override 
on the cmd line:  `gradle -PbuildId=42 build`.  I see it being useful as semver 
"build metadata".


- Anthony


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


On July 26, 2016, 4:56 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50436/
> ---
> 
> (Updated July 26, 2016, 4:56 p.m.)
> 
> 
> Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Extract the product name and org into gradle.properties.
> 
> GEODE-1695: Add buildId to gradle.properties
> 
> 
> GEODE-1695: Use productName to build javadocs
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-assembly/build.gradle 
> 412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
>   geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
>   geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
>   gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
>   gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 
> 
> Diff: https://reviews.apache.org/r/50436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>



Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Dick Cavender

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


Ship it!




Thanks for doing this!

- Dick Cavender


On July 26, 2016, 4:56 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50436/
> ---
> 
> (Updated July 26, 2016, 4:56 p.m.)
> 
> 
> Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Extract the product name and org into gradle.properties.
> 
> GEODE-1695: Add buildId to gradle.properties
> 
> 
> GEODE-1695: Use productName to build javadocs
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-assembly/build.gradle 
> 412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
>   geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
>   geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
>   gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
>   gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 
> 
> Diff: https://reviews.apache.org/r/50436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>



Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Mark Bretl


> On July 26, 2016, 10:02 a.m., Dan Smith wrote:
> > Looks good - but this does mean that the build id will always be 0 unless 
> > someone sets it, whereas before it was the current date. Will that be an 
> > issue?
> 
> Anthony Baker wrote:
> AFAIK, the buildId is only used in GemFireVersion.properties.  That file 
> already has a more useful build date embedded.

The BuildId is used when getting the 'full' version command in GFSH, 'gfsh gfsh 
version --full'

Example:

$ ./gfsh version --full
Java version:   1.0.0-incubating-SNAPSHOT mbretl 072616 2016-07-26 10:59:05 
-0700 javac 1.8.0_65
Native version: native code unavailable
Source revision: b3b2c89098efb71d57c7788b03cf6c933bdd58b1
Source repository: develop
Running on: /172.16.120.55, 8 cpu(s), x86_64 Mac OS X 10.11.5


Before the BuildId was a date reference, it was the SVN revision. Once the 
source was moved to Git, we had to change it. It does hold some value in 
keeping the concept of BuildId, so individual builds can be distinguished from 
running the gfsh command. However, as you can see from the gfsh output, the 
current 'date' value from the BuildId (072616) is redundant to the 'real' date 
next to it. I don't feel like setting it to '0' accomplishes much without 
purpose behind it.

One option could be to use 'git rev-list HEAD --count' to get a more unique 
value. I don't it was added initially since most repositories for RHEL/CentOS 5 
didn't have a version of Git with the rev-list command, which I think came with 
Git version 1.8+.


- Mark


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


On July 26, 2016, 9:56 a.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50436/
> ---
> 
> (Updated July 26, 2016, 9:56 a.m.)
> 
> 
> Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Extract the product name and org into gradle.properties.
> 
> GEODE-1695: Add buildId to gradle.properties
> 
> 
> GEODE-1695: Use productName to build javadocs
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-assembly/build.gradle 
> 412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
>   geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
>   geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
>   gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
>   gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 
> 
> Diff: https://reviews.apache.org/r/50436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>



Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Anthony Baker


> On July 26, 2016, 5:02 p.m., Dan Smith wrote:
> > Looks good - but this does mean that the build id will always be 0 unless 
> > someone sets it, whereas before it was the current date. Will that be an 
> > issue?

AFAIK, the buildId is only used in GemFireVersion.properties.  That file 
already has a more useful build date embedded.


- Anthony


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


On July 26, 2016, 4:56 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50436/
> ---
> 
> (Updated July 26, 2016, 4:56 p.m.)
> 
> 
> Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Extract the product name and org into gradle.properties.
> 
> GEODE-1695: Add buildId to gradle.properties
> 
> 
> GEODE-1695: Use productName to build javadocs
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-assembly/build.gradle 
> 412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
>   geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
>   geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
>   gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
>   gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 
> 
> Diff: https://reviews.apache.org/r/50436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>



Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Dan Smith

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



Looks good - but this does mean that the build id will always be 0 unless 
someone sets it, whereas before it was the current date. Will that be an issue?

- Dan Smith


On July 26, 2016, 4:56 p.m., Anthony Baker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50436/
> ---
> 
> (Updated July 26, 2016, 4:56 p.m.)
> 
> 
> Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Extract the product name and org into gradle.properties.
> 
> GEODE-1695: Add buildId to gradle.properties
> 
> 
> GEODE-1695: Use productName to build javadocs
> 
> 
> Diffs
> -
> 
>   extensions/geode-modules-assembly/build.gradle 
> 412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
>   geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
>   geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
>   gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
>   gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 
> 
> Diff: https://reviews.apache.org/r/50436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anthony Baker
> 
>



Re: Review Request 50436: GEODE-1695: Parameterize build metadata

2016-07-26 Thread Anthony Baker

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

(Updated July 26, 2016, 4:56 p.m.)


Review request for geode, Dick Cavender, Mark Bretl, and Dan Smith.


Repository: geode


Description (updated)
---

Extract the product name and org into gradle.properties.

GEODE-1695: Add buildId to gradle.properties


GEODE-1695: Use productName to build javadocs


Diffs (updated)
-

  extensions/geode-modules-assembly/build.gradle 
412ba096b74b5de4bf6af2b09f66774f4fdf00ed 
  geode-assembly/build.gradle c410f594dac3ac315c0eed2ff8c00d9f061ed473 
  geode-core/build.gradle 030885eae9f681b4438faebe787091929b35ce4a 
  gradle.properties 0fa3765d1b49b7c7f193d8fcb05c328e594a3f11 
  gradle/java.gradle 4acb4da8a8a3da686d6ba10a8a8d57ac7fa375be 

Diff: https://reviews.apache.org/r/50436/diff/


Testing
---


Thanks,

Anthony Baker