Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Anthony Baker]

How do you retrieve git metadata when building from a source distribution (not 
a repo)?  That is why .buildinfo exists *in the source distribution*. 

Anthony

> On Sep 12, 2018, at 4:30 PM, Patrick Rhomberg  wrote:
> 
> Okay.  So that information is definitely coming from the
> GemFireVersion.properties file, which explains this issue.  Either
> reverting the previous GEODE-5600 changes or resolving merge conflicts from
> PR 2457 would address this issue.
> 
> My concern remains about the .buildinfo file, however.Is the .buildinfo
> redundant at this point and should be?  Should it always contain the
> necessary information, with the GemFireVersion.properties file acquiring
> the source information from .buildinfo rather than fetching it again
> itself?  Is .buildinfo a convention in distributions with which I am just
> myself unfamiliar?
> 
> The path we take here is fundamentally linked to how we want to approach
> GEODE-5600, and with PR 2457 currently open, we could choose any of these
> routes to go.
> 
>> On Wed, Sep 12, 2018 at 4:13 PM, Nabarun Nag  wrote:
>> 
>> @patrick
>> if you build geode release branch 1.7.0 "./gradlew clean build
>> -Dskip.tests=true -xdocs -xjavadoc" and start gfsh from
>> geode-assembly/build/install/apache-geode/bin/gfsh
>> And then type `version --full` you get this
>> 
>> gfsh>version --full
>> Build-Date: 2018-09-12 16:07:03 -0700
>> Build-Id: nnag 0
>> Build-Java-Version: 1.8.0_181
>> Build-Platform: Mac OS X 10.13.6 x86_64
>> Product-Name: Apache Geode
>> Product-Version: 1.7.0
>> Source-Date: 2018-09-12 16:07:03 -0700
>> *Source-Repository: unknown*
>> *Source-Revision: unknown*
>> Native version: native code unavailable
>> Running on: /10.118.19.23, 8 cpu(s), x86_64 Mac OS X 10.13.6
>> 
>> As you can notice that Source-Repository and Source-Revision is missing. It
>> should contain the info from the buildinfo file present in
>> geode-assemble/.buildinfo file. It contains the following
>> 
>> #
>> #Wed Sep 12 16:07:56 PDT 2018
>> Source-Date=2018-09-11 15\:56\:48 -0700
>> Source-Revision=c637193aa61abdfd236ae36b6d9a228fc1e84bcd
>> Source-Repository=release/1.7.0
>> 
>> Hope this helps
>> 
>> Regards
>> Nabarun Nag
>> 
>> On Wed, Sep 12, 2018 at 3:51 PM Patrick Rhomberg 
>> wrote:
>> 
>>> I'm happy to work on those reverts, although if Anthony could elaborate
>> on
>>> where exactly the version information was missing, that assuage some of
>> my
>>> own worries as to whether it's the right approach.  It's still not clear
>> to
>>> me where .buildinfo is intended to be consumed.
>>> 
 On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag  wrote:
 
 Yes Alexander, we are still waiting on the build info reverts from
>>> Patrick,
 so, I think that this can be put into release/1.7.0.
 
 Sure Jinmei, you can go ahead and merge the change into release/1.7.0
 branch too when you merge the PR. Please do close the fixed version in
>>> the
 JIRA as 1.7.0
 
 Regards
 Nabarun Nag
 
 
 On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann >> 
 wrote:
 
> While there is a workaround this looks like a highly visible bug
>> with a
> fairly safe fix. I am in favor of merging, since the branch is still
> distressed anyways.
> 
> Other opinions?
> 
> On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao 
>>> wrote:
> 
>> Should we include the fix for GEODE-5727 in the 1.7 release as
>> well?
>> 
>> Without the fix, the command "export cluster-config
> --zip-file-name=x.zip"
>> would fail with NPE, user has to use "export cluster-config
>> --zip-file-name=./x.zip" in order for export to work.
>> 
>> PR for this fix is ready and could be merged soon.
>> 
>> Jinmei
>> 
>> 
>> On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg <
 prhomb...@apache.org>
>> wrote:
>> 
>>> I'm not sure PR 2457 will help with an ignored .buildinfo, but
>> I'm
 not
>> sure
>>> as to why .buildinfo would be getting ignored by anything,
>> either.
>>> 
>>> PR 2457 deals with the still-needs-to-be-renamed
>> GemFireVersion.properties
>>> file and when it is generated.  Previously, it was whenever the
>> git
> index
>>> changed, which was too frequent.  Not it is whenever the source
>> parameters
>>> are passed on the command-line with the build, which has
>> presented
> issues
>>> outside the Concourse pipeline.  PR 2457 splits the difference,
>>> regenerating the file anytime the SHA changes.
>>> 
>>> The only interaction with .buildinfo that I can see is that if
>> the
> build
>>> was run on a machine that was missing git, it would attempt to
>> read
>> values
>>> instead from .buildinfo when creating the
>> GemFireVersion.properties
> file.
>>> 
>>> I guess I don't fully understand the problem Anthony has called
>>> out.
>> Where
>>> is it exactly that information 

Re: [discuss] Should we evaluate at commit messages as part of PR review?

[Jianxia Chen]

There is a good article on this topic that Anthony recommended a while ago:
http://chris.beams.io/posts/git-commit/

Jianxia

On Wed, Sep 12, 2018 at 4:30 PM Michael Stolz  wrote:

> +1 to descriptive commit messages
>
> Done well they will save time on every post commit access to that commit.
>
> --
> Mike Stolz
> Principal Engineer - Gemfire Product Manager
> Mobile: 631-835-4771
>
> On Sep 12, 2018 4:15 PM, "Alexander Murmann"  wrote:
>
> > While our wiki page primarily calls out the 50/74 rule which is
> important,
> > my bigger concern is in that I'd love to see commit messages that explain
> > why a change was made. Sometimes that information is in the reference
> JIRA
> > ticket, but not always. Especially with bug fixes we tend to not explain
> > what actually caused the bug and how the code changes address it. It's
> also
> > nice to minimize moving back and forth between editor and browsing JIRA
> and
> > even better not to have to read 20+ messages long comment threads about a
> > bug in JIRA to find out what the problem was. No hard rule can make sure
> we
> > are doing that right, only human reviewers and care can. I don't think
> it's
> > actually any extra work, since we are already reading the PR description
> > and commit messages when we are doing a PR review. How else can you
> > understand the PR? In fact better commit messages should save time here
> and
> > not steal it.
> >
> > I totally agree that we don't always need a long text. "Fix typo in XYZ
> > output" is totally fine. Both the why and what are obvious.
> >
> > On Wed, Sep 12, 2018 at 4:07 PM, Helena Bales  wrote:
> >
> > > I'm a fan of having useful commit messages. I would prefer this to be
> > > another checkbox in our list of 6 things to do for Pull Requests as
> > opposed
> > > to something that is strictly enforced. There are cases where a simple
> > > one-liner commit message is enough. I personally use commit messages
> > often,
> > > even when looking back at my own work. Additionally, I think it is a
> > useful
> > > exercise as it forces the dev to think back on the original problem and
> > > think about how the solution addresses that.
> > >
> > > tl;dr -- +1 for commit messages
> > >
> > > ~Helena
> > >
> > > On Wed, Sep 12, 2018 at 11:50 AM, Pulkit Chandra 
> > > wrote:
> > >
> > > > Have we thought about git hooks as a way to enforce policy
> > > > https://git-scm.com/book/en/v2/Customizing-Git-An-Example-
> > > > Git-Enforced-Policy
> > > > ?
> > > >
> > > > *Pulkit Chandra*
> > > >
> > > >
> > > > On Wed, Sep 12, 2018 at 2:46 PM Alexander Murmann <
> amurm...@pivotal.io
> > >
> > > > wrote:
> > > >
> > > > > Hi everyone,
> > > > >
> > > > > We have a wiki page
> > > > >  > > Commit+Message+Format
> > > > >
> > > > > that discusses why good commit messages matter and links to a even
> > > better
> > > > > article on the topic. In addition to what's described in those
> > > documents,
> > > > > better commit messages also would make it easier to have good PR
> > > > messages.
> > > > > Good commit and PR messages also provide more context to the
> reviewer
> > > who
> > > > > in turn now can do a better job at reviewing the pull request.
> > > > >
> > > > > Looking at our git log gives me the impression that we aren't
> always
> > > > living
> > > > > up to that standard. In fact we frequently aren't even close.
> > > > >
> > > > > I propose taking clear and well formatted commit messages into
> > account
> > > as
> > > > > part of our PR review process. Lacking commit messages can be just
> as
> > > bad
> > > > > as bad naming in our code.
> > > > >
> > > > > Thoughts?
> > > > >
> > > >
> > >
> >
>

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Anthony Baker]

1) User downloads source distribution [1]
2) User runs `gradle build`
3) User runs `gfsh version --full`

The .buildinfo file contains the information that should go into the 
GemFireVersion.properties file.  Does that help?

Anthony

[1] http://apache.org/dyn/closer.cgi/geode/1.6.0/apache-geode-1.6.0-src.tgz


> On Sep 12, 2018, at 3:51 PM, Patrick Rhomberg  wrote:
> 
> I'm happy to work on those reverts, although if Anthony could elaborate on
> where exactly the version information was missing, that assuage some of my
> own worries as to whether it's the right approach.  It's still not clear to
> me where .buildinfo is intended to be consumed.
> 
> On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag  wrote:
> 
>> Yes Alexander, we are still waiting on the build info reverts from Patrick,
>> so, I think that this can be put into release/1.7.0.
>> 
>> Sure Jinmei, you can go ahead and merge the change into release/1.7.0
>> branch too when you merge the PR. Please do close the fixed version in the
>> JIRA as 1.7.0
>> 
>> Regards
>> Nabarun Nag
>> 
>> 
>> On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann 
>> wrote:
>> 
>>> While there is a workaround this looks like a highly visible bug with a
>>> fairly safe fix. I am in favor of merging, since the branch is still
>>> distressed anyways.
>>> 
>>> Other opinions?
>>> 
>>> On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao  wrote:
>>> 
 Should we include the fix for GEODE-5727 in the 1.7 release as well?
 
 Without the fix, the command "export cluster-config
>>> --zip-file-name=x.zip"
 would fail with NPE, user has to use "export cluster-config
 --zip-file-name=./x.zip" in order for export to work.
 
 PR for this fix is ready and could be merged soon.
 
 Jinmei
 
 
 On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg <
>> prhomb...@apache.org>
 wrote:
 
> I'm not sure PR 2457 will help with an ignored .buildinfo, but I'm
>> not
 sure
> as to why .buildinfo would be getting ignored by anything, either.
> 
> PR 2457 deals with the still-needs-to-be-renamed
 GemFireVersion.properties
> file and when it is generated.  Previously, it was whenever the git
>>> index
> changed, which was too frequent.  Not it is whenever the source
 parameters
> are passed on the command-line with the build, which has presented
>>> issues
> outside the Concourse pipeline.  PR 2457 splits the difference,
> regenerating the file anytime the SHA changes.
> 
> The only interaction with .buildinfo that I can see is that if the
>>> build
> was run on a machine that was missing git, it would attempt to read
 values
> instead from .buildinfo when creating the GemFireVersion.properties
>>> file.
> 
> I guess I don't fully understand the problem Anthony has called out.
 Where
> is it exactly that information previously gathered from .buildinfo is
>>> now
> missing?  And are we certain that it was indeed pulling from
>> .buildinfo
 and
> not the aforementioned GemFireVersion.properties?
> 
> On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann <
>>> amurm...@pivotal.io
> 
> wrote:
> 
>> Hi everyone,
>> 
>> It seems like that PR doesn't address the missing SHA issue either
>>> and
 I
> am
>> not aware of any proposals to properly fix this. How viable is it
>> to
> revert
>> the relevant Gradle build changes on support/1.7?
>> We could continue make the new Gradle approach work with our
>> release
>> process on develop and hopefully release 1.8 with these changes.
>> 
>> Are there any other proposals to unblock this?
>> 
>> On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker 
> wrote:
>> 
>>> Slight clarification—the issue I mentioned is when a user builds
 Geode
>>> from the source distribution.  The source distribution that the
 release
>>> manager creates has the correct .buildinfo file, it’s just
>> ignored
>>> by
> the
>>> build.  In short, the release manager can’t work around the
>>> problem.
>>> 
>>> Does [1] help with this?
>>> 
>>> Anthony
>>> 
>>> [1] https://github.com/apache/geode/pull/2457 <
>> https://github.com/apache/
>>> geode/pull/2457>
>>> 
>>> 
 On Sep 11, 2018, at 3:16 PM, Alexander Murmann <
 amurm...@pivotal.io>
>>> wrote:
 
 What's the consensus on the version info issue Anthony is
>> calling
> out?
>>> Does
 anyone have a proposal for fixing this for this release? Should
> Nabarun
>>> as
 the release manager manually correct this for the release and
>> we
> find a
 permanent solution for 1.8?
 
 On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker <
>>> aba...@pivotal.io
> 
>>> wrote:
 
> Unfortunately it would require a fix to the build—it’s not
>> about
>>> producing
> 

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Alexander Murmann]

+1 for revert

On Wed, Sep 12, 2018 at 4:42 PM, Nabarun Nag  wrote:

> Reverting them on release/1.7.0 will bring it to the previous status quo,
> how all previous releases were done. I don't think anyone will build
> release/1.7.0 repeatedly, hence there is no advantage of making build
> process faster for that branch.
> Whereas on develop a more appropriate solution can be incorporated after
> discussions.
>
> Is it acceptable?
>
> Regards
> Nabarun Nag
>
> On Wed, Sep 12, 2018 at 4:37 PM Patrick Rhomberg 
> wrote:
>
> > Okay.  So that information is definitely coming from the
> > GemFireVersion.properties file, which explains this issue.  Either
> > reverting the previous GEODE-5600 changes or resolving merge conflicts
> from
> > PR 2457 would address this issue.
> >
> > My concern remains about the .buildinfo file, however.Is the
> .buildinfo
> > redundant at this point and should be?  Should it always contain the
> > necessary information, with the GemFireVersion.properties file acquiring
> > the source information from .buildinfo rather than fetching it again
> > itself?  Is .buildinfo a convention in distributions with which I am just
> > myself unfamiliar?
> >
> > The path we take here is fundamentally linked to how we want to approach
> > GEODE-5600, and with PR 2457 currently open, we could choose any of these
> > routes to go.
> >
> > On Wed, Sep 12, 2018 at 4:13 PM, Nabarun Nag  wrote:
> >
> > > @patrick
> > > if you build geode release branch 1.7.0 "./gradlew clean build
> > > -Dskip.tests=true -xdocs -xjavadoc" and start gfsh from
> > > geode-assembly/build/install/apache-geode/bin/gfsh
> > > And then type `version --full` you get this
> > >
> > > gfsh>version --full
> > > Build-Date: 2018-09-12 16:07:03 -0700
> > > Build-Id: nnag 0
> > > Build-Java-Version: 1.8.0_181
> > > Build-Platform: Mac OS X 10.13.6 x86_64
> > > Product-Name: Apache Geode
> > > Product-Version: 1.7.0
> > > Source-Date: 2018-09-12 16:07:03 -0700
> > > *Source-Repository: unknown*
> > > *Source-Revision: unknown*
> > > Native version: native code unavailable
> > > Running on: /10.118.19.23, 8 cpu(s), x86_64 Mac OS X 10.13.6
> > >
> > > As you can notice that Source-Repository and Source-Revision is
> missing.
> > It
> > > should contain the info from the buildinfo file present in
> > > geode-assemble/.buildinfo file. It contains the following
> > >
> > > #
> > > #Wed Sep 12 16:07:56 PDT 2018
> > > Source-Date=2018-09-11 15\:56\:48 -0700
> > > Source-Revision=c637193aa61abdfd236ae36b6d9a228fc1e84bcd
> > > Source-Repository=release/1.7.0
> > >
> > > Hope this helps
> > >
> > > Regards
> > > Nabarun Nag
> > >
> > > On Wed, Sep 12, 2018 at 3:51 PM Patrick Rhomberg  >
> > > wrote:
> > >
> > > > I'm happy to work on those reverts, although if Anthony could
> elaborate
> > > on
> > > > where exactly the version information was missing, that assuage some
> of
> > > my
> > > > own worries as to whether it's the right approach.  It's still not
> > clear
> > > to
> > > > me where .buildinfo is intended to be consumed.
> > > >
> > > > On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag 
> wrote:
> > > >
> > > > > Yes Alexander, we are still waiting on the build info reverts from
> > > > Patrick,
> > > > > so, I think that this can be put into release/1.7.0.
> > > > >
> > > > > Sure Jinmei, you can go ahead and merge the change into
> release/1.7.0
> > > > > branch too when you merge the PR. Please do close the fixed version
> > in
> > > > the
> > > > > JIRA as 1.7.0
> > > > >
> > > > > Regards
> > > > > Nabarun Nag
> > > > >
> > > > >
> > > > > On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann <
> > amurm...@pivotal.io
> > > >
> > > > > wrote:
> > > > >
> > > > > > While there is a workaround this looks like a highly visible bug
> > > with a
> > > > > > fairly safe fix. I am in favor of merging, since the branch is
> > still
> > > > > > distressed anyways.
> > > > > >
> > > > > > Other opinions?
> > > > > >
> > > > > > On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao 
> > > > wrote:
> > > > > >
> > > > > > > Should we include the fix for GEODE-5727 in the 1.7 release as
> > > well?
> > > > > > >
> > > > > > > Without the fix, the command "export cluster-config
> > > > > > --zip-file-name=x.zip"
> > > > > > > would fail with NPE, user has to use "export cluster-config
> > > > > > > --zip-file-name=./x.zip" in order for export to work.
> > > > > > >
> > > > > > > PR for this fix is ready and could be merged soon.
> > > > > > >
> > > > > > > Jinmei
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg <
> > > > > prhomb...@apache.org>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > I'm not sure PR 2457 will help with an ignored .buildinfo,
> but
> > > I'm
> > > > > not
> > > > > > > sure
> > > > > > > > as to why .buildinfo would be getting ignored by anything,
> > > either.
> > > > > > > >
> > > > > > > > PR 2457 deals with the still-needs-to-be-renamed
> > > > > > > GemFireVersion.properties
> > > > 

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Nabarun Nag]

Reverting them on release/1.7.0 will bring it to the previous status quo,
how all previous releases were done. I don't think anyone will build
release/1.7.0 repeatedly, hence there is no advantage of making build
process faster for that branch.
Whereas on develop a more appropriate solution can be incorporated after
discussions.

Is it acceptable?

Regards
Nabarun Nag

On Wed, Sep 12, 2018 at 4:37 PM Patrick Rhomberg 
wrote:

> Okay.  So that information is definitely coming from the
> GemFireVersion.properties file, which explains this issue.  Either
> reverting the previous GEODE-5600 changes or resolving merge conflicts from
> PR 2457 would address this issue.
>
> My concern remains about the .buildinfo file, however.Is the .buildinfo
> redundant at this point and should be?  Should it always contain the
> necessary information, with the GemFireVersion.properties file acquiring
> the source information from .buildinfo rather than fetching it again
> itself?  Is .buildinfo a convention in distributions with which I am just
> myself unfamiliar?
>
> The path we take here is fundamentally linked to how we want to approach
> GEODE-5600, and with PR 2457 currently open, we could choose any of these
> routes to go.
>
> On Wed, Sep 12, 2018 at 4:13 PM, Nabarun Nag  wrote:
>
> > @patrick
> > if you build geode release branch 1.7.0 "./gradlew clean build
> > -Dskip.tests=true -xdocs -xjavadoc" and start gfsh from
> > geode-assembly/build/install/apache-geode/bin/gfsh
> > And then type `version --full` you get this
> >
> > gfsh>version --full
> > Build-Date: 2018-09-12 16:07:03 -0700
> > Build-Id: nnag 0
> > Build-Java-Version: 1.8.0_181
> > Build-Platform: Mac OS X 10.13.6 x86_64
> > Product-Name: Apache Geode
> > Product-Version: 1.7.0
> > Source-Date: 2018-09-12 16:07:03 -0700
> > *Source-Repository: unknown*
> > *Source-Revision: unknown*
> > Native version: native code unavailable
> > Running on: /10.118.19.23, 8 cpu(s), x86_64 Mac OS X 10.13.6
> >
> > As you can notice that Source-Repository and Source-Revision is missing.
> It
> > should contain the info from the buildinfo file present in
> > geode-assemble/.buildinfo file. It contains the following
> >
> > #
> > #Wed Sep 12 16:07:56 PDT 2018
> > Source-Date=2018-09-11 15\:56\:48 -0700
> > Source-Revision=c637193aa61abdfd236ae36b6d9a228fc1e84bcd
> > Source-Repository=release/1.7.0
> >
> > Hope this helps
> >
> > Regards
> > Nabarun Nag
> >
> > On Wed, Sep 12, 2018 at 3:51 PM Patrick Rhomberg 
> > wrote:
> >
> > > I'm happy to work on those reverts, although if Anthony could elaborate
> > on
> > > where exactly the version information was missing, that assuage some of
> > my
> > > own worries as to whether it's the right approach.  It's still not
> clear
> > to
> > > me where .buildinfo is intended to be consumed.
> > >
> > > On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag  wrote:
> > >
> > > > Yes Alexander, we are still waiting on the build info reverts from
> > > Patrick,
> > > > so, I think that this can be put into release/1.7.0.
> > > >
> > > > Sure Jinmei, you can go ahead and merge the change into release/1.7.0
> > > > branch too when you merge the PR. Please do close the fixed version
> in
> > > the
> > > > JIRA as 1.7.0
> > > >
> > > > Regards
> > > > Nabarun Nag
> > > >
> > > >
> > > > On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann <
> amurm...@pivotal.io
> > >
> > > > wrote:
> > > >
> > > > > While there is a workaround this looks like a highly visible bug
> > with a
> > > > > fairly safe fix. I am in favor of merging, since the branch is
> still
> > > > > distressed anyways.
> > > > >
> > > > > Other opinions?
> > > > >
> > > > > On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao 
> > > wrote:
> > > > >
> > > > > > Should we include the fix for GEODE-5727 in the 1.7 release as
> > well?
> > > > > >
> > > > > > Without the fix, the command "export cluster-config
> > > > > --zip-file-name=x.zip"
> > > > > > would fail with NPE, user has to use "export cluster-config
> > > > > > --zip-file-name=./x.zip" in order for export to work.
> > > > > >
> > > > > > PR for this fix is ready and could be merged soon.
> > > > > >
> > > > > > Jinmei
> > > > > >
> > > > > >
> > > > > > On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg <
> > > > prhomb...@apache.org>
> > > > > > wrote:
> > > > > >
> > > > > > > I'm not sure PR 2457 will help with an ignored .buildinfo, but
> > I'm
> > > > not
> > > > > > sure
> > > > > > > as to why .buildinfo would be getting ignored by anything,
> > either.
> > > > > > >
> > > > > > > PR 2457 deals with the still-needs-to-be-renamed
> > > > > > GemFireVersion.properties
> > > > > > > file and when it is generated.  Previously, it was whenever the
> > git
> > > > > index
> > > > > > > changed, which was too frequent.  Not it is whenever the source
> > > > > > parameters
> > > > > > > are passed on the command-line with the build, which has
> > presented
> > > > > issues
> > > > > > > outside the Concourse pipeline.  PR 2457 splits 

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Patrick Rhomberg]

Okay.  So that information is definitely coming from the
GemFireVersion.properties file, which explains this issue.  Either
reverting the previous GEODE-5600 changes or resolving merge conflicts from
PR 2457 would address this issue.

My concern remains about the .buildinfo file, however.Is the .buildinfo
redundant at this point and should be?  Should it always contain the
necessary information, with the GemFireVersion.properties file acquiring
the source information from .buildinfo rather than fetching it again
itself?  Is .buildinfo a convention in distributions with which I am just
myself unfamiliar?

The path we take here is fundamentally linked to how we want to approach
GEODE-5600, and with PR 2457 currently open, we could choose any of these
routes to go.

On Wed, Sep 12, 2018 at 4:13 PM, Nabarun Nag  wrote:

> @patrick
> if you build geode release branch 1.7.0 "./gradlew clean build
> -Dskip.tests=true -xdocs -xjavadoc" and start gfsh from
> geode-assembly/build/install/apache-geode/bin/gfsh
> And then type `version --full` you get this
>
> gfsh>version --full
> Build-Date: 2018-09-12 16:07:03 -0700
> Build-Id: nnag 0
> Build-Java-Version: 1.8.0_181
> Build-Platform: Mac OS X 10.13.6 x86_64
> Product-Name: Apache Geode
> Product-Version: 1.7.0
> Source-Date: 2018-09-12 16:07:03 -0700
> *Source-Repository: unknown*
> *Source-Revision: unknown*
> Native version: native code unavailable
> Running on: /10.118.19.23, 8 cpu(s), x86_64 Mac OS X 10.13.6
>
> As you can notice that Source-Repository and Source-Revision is missing. It
> should contain the info from the buildinfo file present in
> geode-assemble/.buildinfo file. It contains the following
>
> #
> #Wed Sep 12 16:07:56 PDT 2018
> Source-Date=2018-09-11 15\:56\:48 -0700
> Source-Revision=c637193aa61abdfd236ae36b6d9a228fc1e84bcd
> Source-Repository=release/1.7.0
>
> Hope this helps
>
> Regards
> Nabarun Nag
>
> On Wed, Sep 12, 2018 at 3:51 PM Patrick Rhomberg 
> wrote:
>
> > I'm happy to work on those reverts, although if Anthony could elaborate
> on
> > where exactly the version information was missing, that assuage some of
> my
> > own worries as to whether it's the right approach.  It's still not clear
> to
> > me where .buildinfo is intended to be consumed.
> >
> > On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag  wrote:
> >
> > > Yes Alexander, we are still waiting on the build info reverts from
> > Patrick,
> > > so, I think that this can be put into release/1.7.0.
> > >
> > > Sure Jinmei, you can go ahead and merge the change into release/1.7.0
> > > branch too when you merge the PR. Please do close the fixed version in
> > the
> > > JIRA as 1.7.0
> > >
> > > Regards
> > > Nabarun Nag
> > >
> > >
> > > On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann  >
> > > wrote:
> > >
> > > > While there is a workaround this looks like a highly visible bug
> with a
> > > > fairly safe fix. I am in favor of merging, since the branch is still
> > > > distressed anyways.
> > > >
> > > > Other opinions?
> > > >
> > > > On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao 
> > wrote:
> > > >
> > > > > Should we include the fix for GEODE-5727 in the 1.7 release as
> well?
> > > > >
> > > > > Without the fix, the command "export cluster-config
> > > > --zip-file-name=x.zip"
> > > > > would fail with NPE, user has to use "export cluster-config
> > > > > --zip-file-name=./x.zip" in order for export to work.
> > > > >
> > > > > PR for this fix is ready and could be merged soon.
> > > > >
> > > > > Jinmei
> > > > >
> > > > >
> > > > > On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg <
> > > prhomb...@apache.org>
> > > > > wrote:
> > > > >
> > > > > > I'm not sure PR 2457 will help with an ignored .buildinfo, but
> I'm
> > > not
> > > > > sure
> > > > > > as to why .buildinfo would be getting ignored by anything,
> either.
> > > > > >
> > > > > > PR 2457 deals with the still-needs-to-be-renamed
> > > > > GemFireVersion.properties
> > > > > > file and when it is generated.  Previously, it was whenever the
> git
> > > > index
> > > > > > changed, which was too frequent.  Not it is whenever the source
> > > > > parameters
> > > > > > are passed on the command-line with the build, which has
> presented
> > > > issues
> > > > > > outside the Concourse pipeline.  PR 2457 splits the difference,
> > > > > > regenerating the file anytime the SHA changes.
> > > > > >
> > > > > > The only interaction with .buildinfo that I can see is that if
> the
> > > > build
> > > > > > was run on a machine that was missing git, it would attempt to
> read
> > > > > values
> > > > > > instead from .buildinfo when creating the
> GemFireVersion.properties
> > > > file.
> > > > > >
> > > > > > I guess I don't fully understand the problem Anthony has called
> > out.
> > > > > Where
> > > > > > is it exactly that information previously gathered from
> .buildinfo
> > is
> > > > now
> > > > > > missing?  And are we certain that it was indeed pulling from
> > > .buildinfo
> > > > > and
> > > > > > not 

Re: [discuss] Should we evaluate at commit messages as part of PR review?

[Michael Stolz]

+1 to descriptive commit messages

Done well they will save time on every post commit access to that commit.

--
Mike Stolz
Principal Engineer - Gemfire Product Manager
Mobile: 631-835-4771

On Sep 12, 2018 4:15 PM, "Alexander Murmann"  wrote:

> While our wiki page primarily calls out the 50/74 rule which is important,
> my bigger concern is in that I'd love to see commit messages that explain
> why a change was made. Sometimes that information is in the reference JIRA
> ticket, but not always. Especially with bug fixes we tend to not explain
> what actually caused the bug and how the code changes address it. It's also
> nice to minimize moving back and forth between editor and browsing JIRA and
> even better not to have to read 20+ messages long comment threads about a
> bug in JIRA to find out what the problem was. No hard rule can make sure we
> are doing that right, only human reviewers and care can. I don't think it's
> actually any extra work, since we are already reading the PR description
> and commit messages when we are doing a PR review. How else can you
> understand the PR? In fact better commit messages should save time here and
> not steal it.
>
> I totally agree that we don't always need a long text. "Fix typo in XYZ
> output" is totally fine. Both the why and what are obvious.
>
> On Wed, Sep 12, 2018 at 4:07 PM, Helena Bales  wrote:
>
> > I'm a fan of having useful commit messages. I would prefer this to be
> > another checkbox in our list of 6 things to do for Pull Requests as
> opposed
> > to something that is strictly enforced. There are cases where a simple
> > one-liner commit message is enough. I personally use commit messages
> often,
> > even when looking back at my own work. Additionally, I think it is a
> useful
> > exercise as it forces the dev to think back on the original problem and
> > think about how the solution addresses that.
> >
> > tl;dr -- +1 for commit messages
> >
> > ~Helena
> >
> > On Wed, Sep 12, 2018 at 11:50 AM, Pulkit Chandra 
> > wrote:
> >
> > > Have we thought about git hooks as a way to enforce policy
> > > https://git-scm.com/book/en/v2/Customizing-Git-An-Example-
> > > Git-Enforced-Policy
> > > ?
> > >
> > > *Pulkit Chandra*
> > >
> > >
> > > On Wed, Sep 12, 2018 at 2:46 PM Alexander Murmann  >
> > > wrote:
> > >
> > > > Hi everyone,
> > > >
> > > > We have a wiki page
> > > >  > Commit+Message+Format
> > > >
> > > > that discusses why good commit messages matter and links to a even
> > better
> > > > article on the topic. In addition to what's described in those
> > documents,
> > > > better commit messages also would make it easier to have good PR
> > > messages.
> > > > Good commit and PR messages also provide more context to the reviewer
> > who
> > > > in turn now can do a better job at reviewing the pull request.
> > > >
> > > > Looking at our git log gives me the impression that we aren't always
> > > living
> > > > up to that standard. In fact we frequently aren't even close.
> > > >
> > > > I propose taking clear and well formatted commit messages into
> account
> > as
> > > > part of our PR review process. Lacking commit messages can be just as
> > bad
> > > > as bad naming in our code.
> > > >
> > > > Thoughts?
> > > >
> > >
> >
>

Re: [discuss] Should we evaluate at commit messages as part of PR review?

[Alexander Murmann]

While our wiki page primarily calls out the 50/74 rule which is important,
my bigger concern is in that I'd love to see commit messages that explain
why a change was made. Sometimes that information is in the reference JIRA
ticket, but not always. Especially with bug fixes we tend to not explain
what actually caused the bug and how the code changes address it. It's also
nice to minimize moving back and forth between editor and browsing JIRA and
even better not to have to read 20+ messages long comment threads about a
bug in JIRA to find out what the problem was. No hard rule can make sure we
are doing that right, only human reviewers and care can. I don't think it's
actually any extra work, since we are already reading the PR description
and commit messages when we are doing a PR review. How else can you
understand the PR? In fact better commit messages should save time here and
not steal it.

I totally agree that we don't always need a long text. "Fix typo in XYZ
output" is totally fine. Both the why and what are obvious.

On Wed, Sep 12, 2018 at 4:07 PM, Helena Bales  wrote:

> I'm a fan of having useful commit messages. I would prefer this to be
> another checkbox in our list of 6 things to do for Pull Requests as opposed
> to something that is strictly enforced. There are cases where a simple
> one-liner commit message is enough. I personally use commit messages often,
> even when looking back at my own work. Additionally, I think it is a useful
> exercise as it forces the dev to think back on the original problem and
> think about how the solution addresses that.
>
> tl;dr -- +1 for commit messages
>
> ~Helena
>
> On Wed, Sep 12, 2018 at 11:50 AM, Pulkit Chandra 
> wrote:
>
> > Have we thought about git hooks as a way to enforce policy
> > https://git-scm.com/book/en/v2/Customizing-Git-An-Example-
> > Git-Enforced-Policy
> > ?
> >
> > *Pulkit Chandra*
> >
> >
> > On Wed, Sep 12, 2018 at 2:46 PM Alexander Murmann 
> > wrote:
> >
> > > Hi everyone,
> > >
> > > We have a wiki page
> > >  Commit+Message+Format
> > >
> > > that discusses why good commit messages matter and links to a even
> better
> > > article on the topic. In addition to what's described in those
> documents,
> > > better commit messages also would make it easier to have good PR
> > messages.
> > > Good commit and PR messages also provide more context to the reviewer
> who
> > > in turn now can do a better job at reviewing the pull request.
> > >
> > > Looking at our git log gives me the impression that we aren't always
> > living
> > > up to that standard. In fact we frequently aren't even close.
> > >
> > > I propose taking clear and well formatted commit messages into account
> as
> > > part of our PR review process. Lacking commit messages can be just as
> bad
> > > as bad naming in our code.
> > >
> > > Thoughts?
> > >
> >
>

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Nabarun Nag]

@patrick
if you build geode release branch 1.7.0 "./gradlew clean build
-Dskip.tests=true -xdocs -xjavadoc" and start gfsh from
geode-assembly/build/install/apache-geode/bin/gfsh
And then type `version --full` you get this

gfsh>version --full
Build-Date: 2018-09-12 16:07:03 -0700
Build-Id: nnag 0
Build-Java-Version: 1.8.0_181
Build-Platform: Mac OS X 10.13.6 x86_64
Product-Name: Apache Geode
Product-Version: 1.7.0
Source-Date: 2018-09-12 16:07:03 -0700
*Source-Repository: unknown*
*Source-Revision: unknown*
Native version: native code unavailable
Running on: /10.118.19.23, 8 cpu(s), x86_64 Mac OS X 10.13.6

As you can notice that Source-Repository and Source-Revision is missing. It
should contain the info from the buildinfo file present in
geode-assemble/.buildinfo file. It contains the following

#
#Wed Sep 12 16:07:56 PDT 2018
Source-Date=2018-09-11 15\:56\:48 -0700
Source-Revision=c637193aa61abdfd236ae36b6d9a228fc1e84bcd
Source-Repository=release/1.7.0

Hope this helps

Regards
Nabarun Nag

On Wed, Sep 12, 2018 at 3:51 PM Patrick Rhomberg 
wrote:

> I'm happy to work on those reverts, although if Anthony could elaborate on
> where exactly the version information was missing, that assuage some of my
> own worries as to whether it's the right approach.  It's still not clear to
> me where .buildinfo is intended to be consumed.
>
> On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag  wrote:
>
> > Yes Alexander, we are still waiting on the build info reverts from
> Patrick,
> > so, I think that this can be put into release/1.7.0.
> >
> > Sure Jinmei, you can go ahead and merge the change into release/1.7.0
> > branch too when you merge the PR. Please do close the fixed version in
> the
> > JIRA as 1.7.0
> >
> > Regards
> > Nabarun Nag
> >
> >
> > On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann 
> > wrote:
> >
> > > While there is a workaround this looks like a highly visible bug with a
> > > fairly safe fix. I am in favor of merging, since the branch is still
> > > distressed anyways.
> > >
> > > Other opinions?
> > >
> > > On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao 
> wrote:
> > >
> > > > Should we include the fix for GEODE-5727 in the 1.7 release as well?
> > > >
> > > > Without the fix, the command "export cluster-config
> > > --zip-file-name=x.zip"
> > > > would fail with NPE, user has to use "export cluster-config
> > > > --zip-file-name=./x.zip" in order for export to work.
> > > >
> > > > PR for this fix is ready and could be merged soon.
> > > >
> > > > Jinmei
> > > >
> > > >
> > > > On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg <
> > prhomb...@apache.org>
> > > > wrote:
> > > >
> > > > > I'm not sure PR 2457 will help with an ignored .buildinfo, but I'm
> > not
> > > > sure
> > > > > as to why .buildinfo would be getting ignored by anything, either.
> > > > >
> > > > > PR 2457 deals with the still-needs-to-be-renamed
> > > > GemFireVersion.properties
> > > > > file and when it is generated.  Previously, it was whenever the git
> > > index
> > > > > changed, which was too frequent.  Not it is whenever the source
> > > > parameters
> > > > > are passed on the command-line with the build, which has presented
> > > issues
> > > > > outside the Concourse pipeline.  PR 2457 splits the difference,
> > > > > regenerating the file anytime the SHA changes.
> > > > >
> > > > > The only interaction with .buildinfo that I can see is that if the
> > > build
> > > > > was run on a machine that was missing git, it would attempt to read
> > > > values
> > > > > instead from .buildinfo when creating the GemFireVersion.properties
> > > file.
> > > > >
> > > > > I guess I don't fully understand the problem Anthony has called
> out.
> > > > Where
> > > > > is it exactly that information previously gathered from .buildinfo
> is
> > > now
> > > > > missing?  And are we certain that it was indeed pulling from
> > .buildinfo
> > > > and
> > > > > not the aforementioned GemFireVersion.properties?
> > > > >
> > > > > On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann <
> > > amurm...@pivotal.io
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi everyone,
> > > > > >
> > > > > > It seems like that PR doesn't address the missing SHA issue
> either
> > > and
> > > > I
> > > > > am
> > > > > > not aware of any proposals to properly fix this. How viable is it
> > to
> > > > > revert
> > > > > > the relevant Gradle build changes on support/1.7?
> > > > > > We could continue make the new Gradle approach work with our
> > release
> > > > > > process on develop and hopefully release 1.8 with these changes.
> > > > > >
> > > > > > Are there any other proposals to unblock this?
> > > > > >
> > > > > > On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker <
> aba...@pivotal.io>
> > > > > wrote:
> > > > > >
> > > > > > > Slight clarification—the issue I mentioned is when a user
> builds
> > > > Geode
> > > > > > > from the source distribution.  The source distribution that the
> > > > release
> > > > > > > manager creates has the correct 

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #1038 was SUCCESSFUL (with 2456 tests)

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #1038 was successful.
---
Scheduled
2458 tests in total.

https://build.spring.io/browse/SGF-NAG-1038/





--
This message is automatically generated by Atlassian Bamboo

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Patrick Rhomberg]

I'm happy to work on those reverts, although if Anthony could elaborate on
where exactly the version information was missing, that assuage some of my
own worries as to whether it's the right approach.  It's still not clear to
me where .buildinfo is intended to be consumed.

On Wed, Sep 12, 2018 at 3:08 PM, Nabarun Nag  wrote:

> Yes Alexander, we are still waiting on the build info reverts from Patrick,
> so, I think that this can be put into release/1.7.0.
>
> Sure Jinmei, you can go ahead and merge the change into release/1.7.0
> branch too when you merge the PR. Please do close the fixed version in the
> JIRA as 1.7.0
>
> Regards
> Nabarun Nag
>
>
> On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann 
> wrote:
>
> > While there is a workaround this looks like a highly visible bug with a
> > fairly safe fix. I am in favor of merging, since the branch is still
> > distressed anyways.
> >
> > Other opinions?
> >
> > On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao  wrote:
> >
> > > Should we include the fix for GEODE-5727 in the 1.7 release as well?
> > >
> > > Without the fix, the command "export cluster-config
> > --zip-file-name=x.zip"
> > > would fail with NPE, user has to use "export cluster-config
> > > --zip-file-name=./x.zip" in order for export to work.
> > >
> > > PR for this fix is ready and could be merged soon.
> > >
> > > Jinmei
> > >
> > >
> > > On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg <
> prhomb...@apache.org>
> > > wrote:
> > >
> > > > I'm not sure PR 2457 will help with an ignored .buildinfo, but I'm
> not
> > > sure
> > > > as to why .buildinfo would be getting ignored by anything, either.
> > > >
> > > > PR 2457 deals with the still-needs-to-be-renamed
> > > GemFireVersion.properties
> > > > file and when it is generated.  Previously, it was whenever the git
> > index
> > > > changed, which was too frequent.  Not it is whenever the source
> > > parameters
> > > > are passed on the command-line with the build, which has presented
> > issues
> > > > outside the Concourse pipeline.  PR 2457 splits the difference,
> > > > regenerating the file anytime the SHA changes.
> > > >
> > > > The only interaction with .buildinfo that I can see is that if the
> > build
> > > > was run on a machine that was missing git, it would attempt to read
> > > values
> > > > instead from .buildinfo when creating the GemFireVersion.properties
> > file.
> > > >
> > > > I guess I don't fully understand the problem Anthony has called out.
> > > Where
> > > > is it exactly that information previously gathered from .buildinfo is
> > now
> > > > missing?  And are we certain that it was indeed pulling from
> .buildinfo
> > > and
> > > > not the aforementioned GemFireVersion.properties?
> > > >
> > > > On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann <
> > amurm...@pivotal.io
> > > >
> > > > wrote:
> > > >
> > > > > Hi everyone,
> > > > >
> > > > > It seems like that PR doesn't address the missing SHA issue either
> > and
> > > I
> > > > am
> > > > > not aware of any proposals to properly fix this. How viable is it
> to
> > > > revert
> > > > > the relevant Gradle build changes on support/1.7?
> > > > > We could continue make the new Gradle approach work with our
> release
> > > > > process on develop and hopefully release 1.8 with these changes.
> > > > >
> > > > > Are there any other proposals to unblock this?
> > > > >
> > > > > On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker 
> > > > wrote:
> > > > >
> > > > > > Slight clarification—the issue I mentioned is when a user builds
> > > Geode
> > > > > > from the source distribution.  The source distribution that the
> > > release
> > > > > > manager creates has the correct .buildinfo file, it’s just
> ignored
> > by
> > > > the
> > > > > > build.  In short, the release manager can’t work around the
> > problem.
> > > > > >
> > > > > > Does [1] help with this?
> > > > > >
> > > > > > Anthony
> > > > > >
> > > > > > [1] https://github.com/apache/geode/pull/2457 <
> > > > > https://github.com/apache/
> > > > > > geode/pull/2457>
> > > > > >
> > > > > >
> > > > > > > On Sep 11, 2018, at 3:16 PM, Alexander Murmann <
> > > amurm...@pivotal.io>
> > > > > > wrote:
> > > > > > >
> > > > > > > What's the consensus on the version info issue Anthony is
> calling
> > > > out?
> > > > > > Does
> > > > > > > anyone have a proposal for fixing this for this release? Should
> > > > Nabarun
> > > > > > as
> > > > > > > the release manager manually correct this for the release and
> we
> > > > find a
> > > > > > > permanent solution for 1.8?
> > > > > > >
> > > > > > > On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker <
> > aba...@pivotal.io
> > > >
> > > > > > wrote:
> > > > > > >
> > > > > > >> Unfortunately it would require a fix to the build—it’s not
> about
> > > > > > producing
> > > > > > >> the release candidate. It’s when a user builds from the source
> > > > release
> > > > > > that
> > > > > > >> the version info is ignored.
> > > > > > >>
> > > > > > >> Anthony
> > > > > > 

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Nabarun Nag]

Yes Alexander, we are still waiting on the build info reverts from Patrick,
so, I think that this can be put into release/1.7.0.

Sure Jinmei, you can go ahead and merge the change into release/1.7.0
branch too when you merge the PR. Please do close the fixed version in the
JIRA as 1.7.0

Regards
Nabarun Nag


On Wed, Sep 12, 2018 at 2:50 PM Alexander Murmann 
wrote:

> While there is a workaround this looks like a highly visible bug with a
> fairly safe fix. I am in favor of merging, since the branch is still
> distressed anyways.
>
> Other opinions?
>
> On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao  wrote:
>
> > Should we include the fix for GEODE-5727 in the 1.7 release as well?
> >
> > Without the fix, the command "export cluster-config
> --zip-file-name=x.zip"
> > would fail with NPE, user has to use "export cluster-config
> > --zip-file-name=./x.zip" in order for export to work.
> >
> > PR for this fix is ready and could be merged soon.
> >
> > Jinmei
> >
> >
> > On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg 
> > wrote:
> >
> > > I'm not sure PR 2457 will help with an ignored .buildinfo, but I'm not
> > sure
> > > as to why .buildinfo would be getting ignored by anything, either.
> > >
> > > PR 2457 deals with the still-needs-to-be-renamed
> > GemFireVersion.properties
> > > file and when it is generated.  Previously, it was whenever the git
> index
> > > changed, which was too frequent.  Not it is whenever the source
> > parameters
> > > are passed on the command-line with the build, which has presented
> issues
> > > outside the Concourse pipeline.  PR 2457 splits the difference,
> > > regenerating the file anytime the SHA changes.
> > >
> > > The only interaction with .buildinfo that I can see is that if the
> build
> > > was run on a machine that was missing git, it would attempt to read
> > values
> > > instead from .buildinfo when creating the GemFireVersion.properties
> file.
> > >
> > > I guess I don't fully understand the problem Anthony has called out.
> > Where
> > > is it exactly that information previously gathered from .buildinfo is
> now
> > > missing?  And are we certain that it was indeed pulling from .buildinfo
> > and
> > > not the aforementioned GemFireVersion.properties?
> > >
> > > On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann <
> amurm...@pivotal.io
> > >
> > > wrote:
> > >
> > > > Hi everyone,
> > > >
> > > > It seems like that PR doesn't address the missing SHA issue either
> and
> > I
> > > am
> > > > not aware of any proposals to properly fix this. How viable is it to
> > > revert
> > > > the relevant Gradle build changes on support/1.7?
> > > > We could continue make the new Gradle approach work with our release
> > > > process on develop and hopefully release 1.8 with these changes.
> > > >
> > > > Are there any other proposals to unblock this?
> > > >
> > > > On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker 
> > > wrote:
> > > >
> > > > > Slight clarification—the issue I mentioned is when a user builds
> > Geode
> > > > > from the source distribution.  The source distribution that the
> > release
> > > > > manager creates has the correct .buildinfo file, it’s just ignored
> by
> > > the
> > > > > build.  In short, the release manager can’t work around the
> problem.
> > > > >
> > > > > Does [1] help with this?
> > > > >
> > > > > Anthony
> > > > >
> > > > > [1] https://github.com/apache/geode/pull/2457 <
> > > > https://github.com/apache/
> > > > > geode/pull/2457>
> > > > >
> > > > >
> > > > > > On Sep 11, 2018, at 3:16 PM, Alexander Murmann <
> > amurm...@pivotal.io>
> > > > > wrote:
> > > > > >
> > > > > > What's the consensus on the version info issue Anthony is calling
> > > out?
> > > > > Does
> > > > > > anyone have a proposal for fixing this for this release? Should
> > > Nabarun
> > > > > as
> > > > > > the release manager manually correct this for the release and we
> > > find a
> > > > > > permanent solution for 1.8?
> > > > > >
> > > > > > On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker <
> aba...@pivotal.io
> > >
> > > > > wrote:
> > > > > >
> > > > > >> Unfortunately it would require a fix to the build—it’s not about
> > > > > producing
> > > > > >> the release candidate. It’s when a user builds from the source
> > > release
> > > > > that
> > > > > >> the version info is ignored.
> > > > > >>
> > > > > >> Anthony
> > > > > >>
> > > > > >>> On Sep 10, 2018, at 10:02 AM, Nabarun Nag 
> > wrote:
> > > > > >>>
> > > > > >>> Hello Anthony,
> > > > > >>>
> > > > > >>> I plan to do that while creating the release candidate. If
> there
> > > are
> > > > no
> > > > > >>> concerns raised on the release branch, I will start with the
> > > process
> > > > > >> soon.
> > > > > >>>
> > > > > >>> Regards
> > > > > >>> Nabarun Nag
> > > > > >>>
> > > > >  On Mon, Sep 10, 2018 at 8:51 AM Anthony Baker <
> > aba...@pivotal.io>
> > > > > >> wrote:
> > > > > 
> > > > >  Looks good Naba!  Only thing I see right now is that building
> > from
> > > > the
> > 

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Alexander Murmann]

While there is a workaround this looks like a highly visible bug with a
fairly safe fix. I am in favor of merging, since the branch is still
distressed anyways.

Other opinions?

On Wed, Sep 12, 2018 at 2:29 PM, Jinmei Liao  wrote:

> Should we include the fix for GEODE-5727 in the 1.7 release as well?
>
> Without the fix, the command "export cluster-config --zip-file-name=x.zip"
> would fail with NPE, user has to use "export cluster-config
> --zip-file-name=./x.zip" in order for export to work.
>
> PR for this fix is ready and could be merged soon.
>
> Jinmei
>
>
> On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg 
> wrote:
>
> > I'm not sure PR 2457 will help with an ignored .buildinfo, but I'm not
> sure
> > as to why .buildinfo would be getting ignored by anything, either.
> >
> > PR 2457 deals with the still-needs-to-be-renamed
> GemFireVersion.properties
> > file and when it is generated.  Previously, it was whenever the git index
> > changed, which was too frequent.  Not it is whenever the source
> parameters
> > are passed on the command-line with the build, which has presented issues
> > outside the Concourse pipeline.  PR 2457 splits the difference,
> > regenerating the file anytime the SHA changes.
> >
> > The only interaction with .buildinfo that I can see is that if the build
> > was run on a machine that was missing git, it would attempt to read
> values
> > instead from .buildinfo when creating the GemFireVersion.properties file.
> >
> > I guess I don't fully understand the problem Anthony has called out.
> Where
> > is it exactly that information previously gathered from .buildinfo is now
> > missing?  And are we certain that it was indeed pulling from .buildinfo
> and
> > not the aforementioned GemFireVersion.properties?
> >
> > On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann  >
> > wrote:
> >
> > > Hi everyone,
> > >
> > > It seems like that PR doesn't address the missing SHA issue either and
> I
> > am
> > > not aware of any proposals to properly fix this. How viable is it to
> > revert
> > > the relevant Gradle build changes on support/1.7?
> > > We could continue make the new Gradle approach work with our release
> > > process on develop and hopefully release 1.8 with these changes.
> > >
> > > Are there any other proposals to unblock this?
> > >
> > > On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker 
> > wrote:
> > >
> > > > Slight clarification—the issue I mentioned is when a user builds
> Geode
> > > > from the source distribution.  The source distribution that the
> release
> > > > manager creates has the correct .buildinfo file, it’s just ignored by
> > the
> > > > build.  In short, the release manager can’t work around the problem.
> > > >
> > > > Does [1] help with this?
> > > >
> > > > Anthony
> > > >
> > > > [1] https://github.com/apache/geode/pull/2457 <
> > > https://github.com/apache/
> > > > geode/pull/2457>
> > > >
> > > >
> > > > > On Sep 11, 2018, at 3:16 PM, Alexander Murmann <
> amurm...@pivotal.io>
> > > > wrote:
> > > > >
> > > > > What's the consensus on the version info issue Anthony is calling
> > out?
> > > > Does
> > > > > anyone have a proposal for fixing this for this release? Should
> > Nabarun
> > > > as
> > > > > the release manager manually correct this for the release and we
> > find a
> > > > > permanent solution for 1.8?
> > > > >
> > > > > On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker  >
> > > > wrote:
> > > > >
> > > > >> Unfortunately it would require a fix to the build—it’s not about
> > > > producing
> > > > >> the release candidate. It’s when a user builds from the source
> > release
> > > > that
> > > > >> the version info is ignored.
> > > > >>
> > > > >> Anthony
> > > > >>
> > > > >>> On Sep 10, 2018, at 10:02 AM, Nabarun Nag 
> wrote:
> > > > >>>
> > > > >>> Hello Anthony,
> > > > >>>
> > > > >>> I plan to do that while creating the release candidate. If there
> > are
> > > no
> > > > >>> concerns raised on the release branch, I will start with the
> > process
> > > > >> soon.
> > > > >>>
> > > > >>> Regards
> > > > >>> Nabarun Nag
> > > > >>>
> > > >  On Mon, Sep 10, 2018 at 8:51 AM Anthony Baker <
> aba...@pivotal.io>
> > > > >> wrote:
> > > > 
> > > >  Looks good Naba!  Only thing I see right now is that building
> from
> > > the
> > > >  source distribution does not use the .buildinfo file, leaving
> the
> > > > >> version
> > > >  string empty.
> > > > 
> > > >  Anthony
> > > > 
> > > > 
> > > > > On Sep 7, 2018, at 9:15 AM, Nabarun Nag 
> wrote:
> > > > >
> > > > > CORRECTION: if '*no*' concerns are raised, we will start with
> the
> > > > >> voting
> > > > > for the release candidate soon.
> > > > >
> > > > > Regrads
> > > > > Nabarun Nag
> > > > >
> > > > >> On Fri, Sep 7, 2018 at 9:08 AM Nabarun Nag 
> > > wrote:
> > > > >>
> > > > >> Hello Geode Dev Community,
> > > > >>
> > > > >> We have created a new release branch for Apache Geode 1.7.0 

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Jinmei Liao]

Should we include the fix for GEODE-5727 in the 1.7 release as well?

Without the fix, the command "export cluster-config --zip-file-name=x.zip"
would fail with NPE, user has to use "export cluster-config
--zip-file-name=./x.zip" in order for export to work.

PR for this fix is ready and could be merged soon.

Jinmei


On Wed, Sep 12, 2018 at 11:12 AM Patrick Rhomberg 
wrote:

> I'm not sure PR 2457 will help with an ignored .buildinfo, but I'm not sure
> as to why .buildinfo would be getting ignored by anything, either.
>
> PR 2457 deals with the still-needs-to-be-renamed GemFireVersion.properties
> file and when it is generated.  Previously, it was whenever the git index
> changed, which was too frequent.  Not it is whenever the source parameters
> are passed on the command-line with the build, which has presented issues
> outside the Concourse pipeline.  PR 2457 splits the difference,
> regenerating the file anytime the SHA changes.
>
> The only interaction with .buildinfo that I can see is that if the build
> was run on a machine that was missing git, it would attempt to read values
> instead from .buildinfo when creating the GemFireVersion.properties file.
>
> I guess I don't fully understand the problem Anthony has called out.  Where
> is it exactly that information previously gathered from .buildinfo is now
> missing?  And are we certain that it was indeed pulling from .buildinfo and
> not the aforementioned GemFireVersion.properties?
>
> On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann 
> wrote:
>
> > Hi everyone,
> >
> > It seems like that PR doesn't address the missing SHA issue either and I
> am
> > not aware of any proposals to properly fix this. How viable is it to
> revert
> > the relevant Gradle build changes on support/1.7?
> > We could continue make the new Gradle approach work with our release
> > process on develop and hopefully release 1.8 with these changes.
> >
> > Are there any other proposals to unblock this?
> >
> > On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker 
> wrote:
> >
> > > Slight clarification—the issue I mentioned is when a user builds Geode
> > > from the source distribution.  The source distribution that the release
> > > manager creates has the correct .buildinfo file, it’s just ignored by
> the
> > > build.  In short, the release manager can’t work around the problem.
> > >
> > > Does [1] help with this?
> > >
> > > Anthony
> > >
> > > [1] https://github.com/apache/geode/pull/2457 <
> > https://github.com/apache/
> > > geode/pull/2457>
> > >
> > >
> > > > On Sep 11, 2018, at 3:16 PM, Alexander Murmann 
> > > wrote:
> > > >
> > > > What's the consensus on the version info issue Anthony is calling
> out?
> > > Does
> > > > anyone have a proposal for fixing this for this release? Should
> Nabarun
> > > as
> > > > the release manager manually correct this for the release and we
> find a
> > > > permanent solution for 1.8?
> > > >
> > > > On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker 
> > > wrote:
> > > >
> > > >> Unfortunately it would require a fix to the build—it’s not about
> > > producing
> > > >> the release candidate. It’s when a user builds from the source
> release
> > > that
> > > >> the version info is ignored.
> > > >>
> > > >> Anthony
> > > >>
> > > >>> On Sep 10, 2018, at 10:02 AM, Nabarun Nag  wrote:
> > > >>>
> > > >>> Hello Anthony,
> > > >>>
> > > >>> I plan to do that while creating the release candidate. If there
> are
> > no
> > > >>> concerns raised on the release branch, I will start with the
> process
> > > >> soon.
> > > >>>
> > > >>> Regards
> > > >>> Nabarun Nag
> > > >>>
> > >  On Mon, Sep 10, 2018 at 8:51 AM Anthony Baker 
> > > >> wrote:
> > > 
> > >  Looks good Naba!  Only thing I see right now is that building from
> > the
> > >  source distribution does not use the .buildinfo file, leaving the
> > > >> version
> > >  string empty.
> > > 
> > >  Anthony
> > > 
> > > 
> > > > On Sep 7, 2018, at 9:15 AM, Nabarun Nag  wrote:
> > > >
> > > > CORRECTION: if '*no*' concerns are raised, we will start with the
> > > >> voting
> > > > for the release candidate soon.
> > > >
> > > > Regrads
> > > > Nabarun Nag
> > > >
> > > >> On Fri, Sep 7, 2018 at 9:08 AM Nabarun Nag 
> > wrote:
> > > >>
> > > >> Hello Geode Dev Community,
> > > >>
> > > >> We have created a new release branch for Apache Geode 1.7.0 -
> > > >> "release/1.7.0"
> > > >>
> > > >> Previous branch was deleted and has been replaced with a fresh
> > one.
> > > >>
> > > >> Please do review and raise any concern with the release branch.
> > > >>
> > > >> If concerns are raised, we will start with the voting for the
> > > release
> > > >> candidate soon.
> > > >>
> > > >> Regards
> > > >> Nabarun Nag
> > > >>
> > > 
> > >  --
> > > >>> Regards
> > > >>> Nabarun Nag
> > > >>
> > >
> > >
> >
>


-- 
Cheers

Jinmei

Re: [discuss] Should we evaluate at commit messages as part of PR review?

[Pulkit Chandra]

Have we thought about git hooks as a way to enforce policy
https://git-scm.com/book/en/v2/Customizing-Git-An-Example-Git-Enforced-Policy
?

*Pulkit Chandra*


On Wed, Sep 12, 2018 at 2:46 PM Alexander Murmann 
wrote:

> Hi everyone,
>
> We have a wiki page
> 
> that discusses why good commit messages matter and links to a even better
> article on the topic. In addition to what's described in those documents,
> better commit messages also would make it easier to have good PR messages.
> Good commit and PR messages also provide more context to the reviewer who
> in turn now can do a better job at reviewing the pull request.
>
> Looking at our git log gives me the impression that we aren't always living
> up to that standard. In fact we frequently aren't even close.
>
> I propose taking clear and well formatted commit messages into account as
> part of our PR review process. Lacking commit messages can be just as bad
> as bad naming in our code.
>
> Thoughts?
>

[discuss] Should we evaluate at commit messages as part of PR review?

[Alexander Murmann]

Hi everyone,

We have a wiki page

that discusses why good commit messages matter and links to a even better
article on the topic. In addition to what's described in those documents,
better commit messages also would make it easier to have good PR messages.
Good commit and PR messages also provide more context to the reviewer who
in turn now can do a better job at reviewing the pull request.

Looking at our git log gives me the impression that we aren't always living
up to that standard. In fact we frequently aren't even close.

I propose taking clear and well formatted commit messages into account as
part of our PR review process. Lacking commit messages can be just as bad
as bad naming in our code.

Thoughts?

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Patrick Rhomberg]

I'm not sure PR 2457 will help with an ignored .buildinfo, but I'm not sure
as to why .buildinfo would be getting ignored by anything, either.

PR 2457 deals with the still-needs-to-be-renamed GemFireVersion.properties
file and when it is generated.  Previously, it was whenever the git index
changed, which was too frequent.  Not it is whenever the source parameters
are passed on the command-line with the build, which has presented issues
outside the Concourse pipeline.  PR 2457 splits the difference,
regenerating the file anytime the SHA changes.

The only interaction with .buildinfo that I can see is that if the build
was run on a machine that was missing git, it would attempt to read values
instead from .buildinfo when creating the GemFireVersion.properties file.

I guess I don't fully understand the problem Anthony has called out.  Where
is it exactly that information previously gathered from .buildinfo is now
missing?  And are we certain that it was indeed pulling from .buildinfo and
not the aforementioned GemFireVersion.properties?

On Wed, Sep 12, 2018 at 10:11 AM, Alexander Murmann 
wrote:

> Hi everyone,
>
> It seems like that PR doesn't address the missing SHA issue either and I am
> not aware of any proposals to properly fix this. How viable is it to revert
> the relevant Gradle build changes on support/1.7?
> We could continue make the new Gradle approach work with our release
> process on develop and hopefully release 1.8 with these changes.
>
> Are there any other proposals to unblock this?
>
> On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker  wrote:
>
> > Slight clarification—the issue I mentioned is when a user builds Geode
> > from the source distribution.  The source distribution that the release
> > manager creates has the correct .buildinfo file, it’s just ignored by the
> > build.  In short, the release manager can’t work around the problem.
> >
> > Does [1] help with this?
> >
> > Anthony
> >
> > [1] https://github.com/apache/geode/pull/2457 <
> https://github.com/apache/
> > geode/pull/2457>
> >
> >
> > > On Sep 11, 2018, at 3:16 PM, Alexander Murmann 
> > wrote:
> > >
> > > What's the consensus on the version info issue Anthony is calling out?
> > Does
> > > anyone have a proposal for fixing this for this release? Should Nabarun
> > as
> > > the release manager manually correct this for the release and we find a
> > > permanent solution for 1.8?
> > >
> > > On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker 
> > wrote:
> > >
> > >> Unfortunately it would require a fix to the build—it’s not about
> > producing
> > >> the release candidate. It’s when a user builds from the source release
> > that
> > >> the version info is ignored.
> > >>
> > >> Anthony
> > >>
> > >>> On Sep 10, 2018, at 10:02 AM, Nabarun Nag  wrote:
> > >>>
> > >>> Hello Anthony,
> > >>>
> > >>> I plan to do that while creating the release candidate. If there are
> no
> > >>> concerns raised on the release branch, I will start with the process
> > >> soon.
> > >>>
> > >>> Regards
> > >>> Nabarun Nag
> > >>>
> >  On Mon, Sep 10, 2018 at 8:51 AM Anthony Baker 
> > >> wrote:
> > 
> >  Looks good Naba!  Only thing I see right now is that building from
> the
> >  source distribution does not use the .buildinfo file, leaving the
> > >> version
> >  string empty.
> > 
> >  Anthony
> > 
> > 
> > > On Sep 7, 2018, at 9:15 AM, Nabarun Nag  wrote:
> > >
> > > CORRECTION: if '*no*' concerns are raised, we will start with the
> > >> voting
> > > for the release candidate soon.
> > >
> > > Regrads
> > > Nabarun Nag
> > >
> > >> On Fri, Sep 7, 2018 at 9:08 AM Nabarun Nag 
> wrote:
> > >>
> > >> Hello Geode Dev Community,
> > >>
> > >> We have created a new release branch for Apache Geode 1.7.0 -
> > >> "release/1.7.0"
> > >>
> > >> Previous branch was deleted and has been replaced with a fresh
> one.
> > >>
> > >> Please do review and raise any concern with the release branch.
> > >>
> > >> If concerns are raised, we will start with the voting for the
> > release
> > >> candidate soon.
> > >>
> > >> Regards
> > >> Nabarun Nag
> > >>
> > 
> >  --
> > >>> Regards
> > >>> Nabarun Nag
> > >>
> >
> >
>

Re: [DISCUSS] and the NEW Apache Geode 1.7.0 release branch has been created

[Alexander Murmann]

Hi everyone,

It seems like that PR doesn't address the missing SHA issue either and I am
not aware of any proposals to properly fix this. How viable is it to revert
the relevant Gradle build changes on support/1.7?
We could continue make the new Gradle approach work with our release
process on develop and hopefully release 1.8 with these changes.

Are there any other proposals to unblock this?

On Tue, Sep 11, 2018 at 5:41 PM, Anthony Baker  wrote:

> Slight clarification—the issue I mentioned is when a user builds Geode
> from the source distribution.  The source distribution that the release
> manager creates has the correct .buildinfo file, it’s just ignored by the
> build.  In short, the release manager can’t work around the problem.
>
> Does [1] help with this?
>
> Anthony
>
> [1] https://github.com/apache/geode/pull/2457  geode/pull/2457>
>
>
> > On Sep 11, 2018, at 3:16 PM, Alexander Murmann 
> wrote:
> >
> > What's the consensus on the version info issue Anthony is calling out?
> Does
> > anyone have a proposal for fixing this for this release? Should Nabarun
> as
> > the release manager manually correct this for the release and we find a
> > permanent solution for 1.8?
> >
> > On Mon, Sep 10, 2018 at 12:33 PM, Anthony Baker 
> wrote:
> >
> >> Unfortunately it would require a fix to the build—it’s not about
> producing
> >> the release candidate. It’s when a user builds from the source release
> that
> >> the version info is ignored.
> >>
> >> Anthony
> >>
> >>> On Sep 10, 2018, at 10:02 AM, Nabarun Nag  wrote:
> >>>
> >>> Hello Anthony,
> >>>
> >>> I plan to do that while creating the release candidate. If there are no
> >>> concerns raised on the release branch, I will start with the process
> >> soon.
> >>>
> >>> Regards
> >>> Nabarun Nag
> >>>
>  On Mon, Sep 10, 2018 at 8:51 AM Anthony Baker 
> >> wrote:
> 
>  Looks good Naba!  Only thing I see right now is that building from the
>  source distribution does not use the .buildinfo file, leaving the
> >> version
>  string empty.
> 
>  Anthony
> 
> 
> > On Sep 7, 2018, at 9:15 AM, Nabarun Nag  wrote:
> >
> > CORRECTION: if '*no*' concerns are raised, we will start with the
> >> voting
> > for the release candidate soon.
> >
> > Regrads
> > Nabarun Nag
> >
> >> On Fri, Sep 7, 2018 at 9:08 AM Nabarun Nag  wrote:
> >>
> >> Hello Geode Dev Community,
> >>
> >> We have created a new release branch for Apache Geode 1.7.0 -
> >> "release/1.7.0"
> >>
> >> Previous branch was deleted and has been replaced with a fresh one.
> >>
> >> Please do review and raise any concern with the release branch.
> >>
> >> If concerns are raised, we will start with the voting for the
> release
> >> candidate soon.
> >>
> >> Regards
> >> Nabarun Nag
> >>
> 
>  --
> >>> Regards
> >>> Nabarun Nag
> >>
>
>

Re: please give me (bburcham) permission to assign himself Jira tickets

[Dan Smith]

Done! You should have access now.

-Dan

On Wed, Sep 12, 2018 at 10:05 AM, Bill Burcham  wrote:

> I made a ticket and would like to assign it to myself. Please let me.
>
> -Bill
>

please give me (bburcham) permission to assign himself Jira tickets

[Bill Burcham]

I made a ticket and would like to assign it to myself. Please let me.

-Bill