Re: Issues with TransactionIds managed by CacheTransactionManager in C++ native client

[Alberto Gomez]

Hi Blake,

Are you sure that the TransactionId is always set to -1?

We have used the C++ native client transactions and they apparently worked.

/Alberto G.


From: Blake Bender 
Sent: Friday, December 13, 2019 6:17 PM
To: dev@geode.apache.org 
Subject: Re: Issues with TransactionIds managed by CacheTransactionManager in 
C++ native client

Transactions are an area of the codebase I've never dealt with, so I'm sure
most/all of what you mention is true.  In particular, the only thing I know
about TransactionId is that it's always set to -1, so functionally it's
essentially useless.  I'm not certain what all of the implications will be
if suddenly we ascribe meaning to it, so I'll let some folks with more
native client history than I chime in.

Thanks,

Blake


On Fri, Dec 13, 2019 at 3:15 AM Alberto Gomez 
wrote:

> Hi,
>
> I have created a ticket with some issues I have found related to
> TransactionIds managed by CacheTransactionManager in the C++ native client.
>
> https://issues.apache.org/jira/browse/GEODE-7573
>
> In it, I also propose some solutions to the issues found.
>
> I'd appreciate if someone could review the analysis to see if I am in the
> right track or if I am missing something.
>
> Thanks in advance,
>
> Alberto
>

Re: [DISCUSS] Proposal to require linear commit history on develop

[Bruce Schuchardt]

I agree with Jake.  I would go further by saying that I see very little 
merit in this proposal.  I think we're getting more and more 
bureaucratic in our process and that it stifles productivity.  I was 
recently forced to spend three days fixing tests in which I had changed 
an import statement before they would pass stress testing.  I'm glad the 
tests now pass reliably but I was very frustrated by the process.


On 12/19/19 4:49 PM, Jacob Barrett wrote:

I’m in agreement with Dan. Changes to the infrastructure to flat out prevent 
things that should be self policing is annoying. This PR review lock we have 
had already cost us valuable time waiting for PR pipelines to pass that have no 
relevance to the commit, like CI work: I’d hat to see yet another process 
enforced that Kees us from getting work done when necessary.

-Jake



On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:

-1 to (1) and (2).

I think merge commits are appropriate in certain circumstances, so I don't
think we should make a blanket restriction. In fact I think our release
process involves some merges.

I think setting standards on what is reasonable to be an individual commit
will do a lot more to clean up our history than blocking merges. We'd
rather not see commits like "Spotless Apply" in the history, but if
reasonably separate and well written commits come in as part of a merge, I
think that's fine.

-Dan


On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao  wrote:

+1


On Thu, Dec 19, 2019, 4:05 PM Owen Nichols  wrote:

I’d like to advance this topic from an informal request/discussion to a
discussion of a concrete proposal.

To recap, it sounds like there is general agreement that commit history

on

develop should be linear (no merge commits), and that the biggest

obstacle

to this is that the PR merge button in GitHub creates a merge commit by
default.

I propose the following changes:
1) Change GitHub settings to remove the ability to create a merge commit.
This will make Squash & Merge the default.

2) Change GitHub settings to require linear history on develop.  This
prevents merge commits via command-line (not recommended, but wiki still
has instructions for merging PRs this way).

3) Update the PR template to change the text "Is your initial

contribution

a single, squashed commit” to “Is your initial contribution squashed for
ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
‘fix’ commit)"

For clarity, I am proposing no-change in the following areas:
i) Leave Rebase & Merge as an option for PRs that have been structured to
benefit from it (this can make it easier in a bisect to see whether the
refactoring or the “fix” broke something).
ii) Leave existing wording in the wiki as-is [stating that squashing is
preferred].


Please comment via this email thread.
-Owen




On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:

I think it's already resolved Udo ;)

Here's the problem, if I fixup a dunit test by removing all uses of

"this."

and I rename the dunit test, then git doesn't remember that the file

is a

rename -- it forever afterwards interprets it as a new file that I

created

if I touch more than 50% of the lines (which "this." can easily do). If

we

squash two commits: the rename and the cleanup of that dunit test --

then

we effectively lose the history of that file and it shows that I

created

a

new file.

Also for the record, I've been working on Geode since the beginning

and I

was never made aware of this change in our process. I never voted on

it.

I'm not a big fan of changing various details in our process every

single

week. It's very easy to miss these discussions unless someone points it

out

to me.

On Mon, Dec 16, 2019 at 10:34 AM Udo Kohlmeyer 
wrote:


I'm not sure what this discussion is about... WE, as a community, have
agreed in common practices, in two place no less...

1) Quoting our PR template


 For all changes:

*

   Is there a JIRA ticket associated with this PR? Is it referenced in
   the commit message?

*

   Has your PR been rebased against the latest commit within the

target

   branch (typically|develop|)?

*

   ***Is your initial contribution a single, squashed commit?*

*

   Does|gradlew build|run cleanly?

*

   Have you written or updated unit tests to verify your changes?

*

   If adding new dependencies to the code, are these dependencies
   licensed in a way that is compatible for inclusion underASF 2.0
   ?

On our PR template we call out that the initial PR commit should be
squashed.

2)

https://cwiki.apache.org/confluence/display/GEODE/Code+contributions

Re: [DISCUSS] Proposal to require linear commit history on develop

[Jinmei Liao]

Yes, I got freaked out somehow this morning by thinking I had accidentally
clicked the wrong button to include all the individual commits in my PR to
the develop branch, (luckily it's just a wrong view). I would think disable
that button would be a good idea.

On Thu, Dec 19, 2019 at 5:04 PM Nabarun Nag  wrote:

> Hi all,
>
> This is not a blanket restriction.
> My suggestion, like last time, lets try it out and this time we don't even
> need Apache Infra to step in. Feels like it has been requested in multiple
> INFRA JIRAs by so many Apache projects to enable only the squash merge
> button and disable the others, that this setting has been moved to a simple
> entry in .asf.yml file in root git repository
>
> Again this is a setting only for the develop branch. And can be reversed
> with a simple commit.
>
> I agree too much policing is a bad thing, but I want to look at it as a
> safeguard. I have spoken to multiple developers who have mentioned they
> clicked the wrong button in a hurry or the page refreshed and the commit
> got merged in a wrong way.
>
> Regards
> Naba
>
> Reference :
>
> https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories
> Merge buttons
>
> Projects can enable/disable the "merge PR" button in the GitHub UI and
> configure which actions should be allowed by adding the following
> configuration (or derivatives thereof):
> github:
>   enabled_merge_buttons:
> # enable squash button:
> squash:  true
> # enable merge button:
> merge:   true
> # disable rebase button:
> rebase:  false
>
>
> On Thu, Dec 19, 2019 at 4:54 PM Owen Nichols  wrote:
>
> > This proposal in no way prevents you from getting work done.  Squash is
> > always enabled and always the most acceptable way to bring changes to
> > develop.
> >
> > Please start a separate thread if you would like to revisit the community
> > decision to require passing PR checks.
> >
> > > On Dec 19, 2019, at 4:49 PM, Jacob Barrett 
> wrote:
> > >
> > > I’m in agreement with Dan. Changes to the infrastructure to flat out
> > prevent things that should be self policing is annoying. This PR review
> > lock we have had already cost us valuable time waiting for PR pipelines
> to
> > pass that have no relevance to the commit, like CI work: I’d hat to see
> yet
> > another process enforced that Kees us from getting work done when
> necessary.
> > >
> > > -Jake
> > >
> > >
> > >> On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> > >>
> > >> -1 to (1) and (2).
> > >>
> > >> I think merge commits are appropriate in certain circumstances, so I
> > don't
> > >> think we should make a blanket restriction. In fact I think our
> release
> > >> process involves some merges.
> > >>
> > >> I think setting standards on what is reasonable to be an individual
> > commit
> > >> will do a lot more to clean up our history than blocking merges. We'd
> > >> rather not see commits like "Spotless Apply" in the history, but if
> > >> reasonably separate and well written commits come in as part of a
> > merge, I
> > >> think that's fine.
> > >>
> > >> -Dan
> > >>
> > >>> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
> wrote:
> > >>>
> > >>> +1
> > >>>
> >  On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 
> > wrote:
> > 
> >  I’d like to advance this topic from an informal request/discussion
> to
> > a
> >  discussion of a concrete proposal.
> > 
> >  To recap, it sounds like there is general agreement that commit
> > history
> > >>> on
> >  develop should be linear (no merge commits), and that the biggest
> > >>> obstacle
> >  to this is that the PR merge button in GitHub creates a merge commit
> > by
> >  default.
> > 
> >  I propose the following changes:
> >  1) Change GitHub settings to remove the ability to create a merge
> > commit.
> >  This will make Squash & Merge the default.
> > 
> >  2) Change GitHub settings to require linear history on develop.
> This
> >  prevents merge commits via command-line (not recommended, but wiki
> > still
> >  has instructions for merging PRs this way).
> > 
> >  3) Update the PR template to change the text "Is your initial
> > >>> contribution
> >  a single, squashed commit” to “Is your initial contribution squashed
> > for
> >  ease of review (e.g. a single commit, or a ‘refactoring’ commit
> plus a
> >  ‘fix’ commit)"
> > 
> >  For clarity, I am proposing no-change in the following areas:
> >  i) Leave Rebase & Merge as an option for PRs that have been
> > structured to
> >  benefit from it (this can make it easier in a bisect to see whether
> > the
> >  refactoring or the “fix” broke something).
> >  ii) Leave existing wording in the wiki as-is [stating that squashing
> > is
> >  preferred].
> > 
> > 
> >  Please comment via this email thread.
> >  -Owen
> > 
> > 
> > 
> > > On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:
> > 

Re: [DISCUSS] Proposal to require linear commit history on develop

[Owen Nichols]

Hi Bruce, this proposal will not waste a single second of your time.  It just 
prevents people from accidentally pressing a button that we have already agreed 
should never be pressed, but because we never configured our GitHub to match 
our stated policy, is currently the default.

However, it will save a lot of time and frustration for anyone needing to 
bisect failures, revert, or cherry-pick changes, which has merit even if you 
personally never do any of those three things.

Please start a separate thread if you would like to revisit the community 
decision to require passing PR checks.

> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt  wrote:
> 
> I agree with Jake.  I would go further by saying that I see very little merit 
> in this proposal.  I think we're getting more and more bureaucratic in our 
> process and that it stifles productivity.  I was recently forced to spend 
> three days fixing tests in which I had changed an import statement before 
> they would pass stress testing.  I'm glad the tests now pass reliably but I 
> was very frustrated by the process.
> 
> On 12/19/19 4:49 PM, Jacob Barrett wrote:
>> I’m in agreement with Dan. Changes to the infrastructure to flat out prevent 
>> things that should be self policing is annoying. This PR review lock we have 
>> had already cost us valuable time waiting for PR pipelines to pass that have 
>> no relevance to the commit, like CI work: I’d hat to see yet another process 
>> enforced that Kees us from getting work done when necessary.
>> 
>> -Jake
>> 
>> 
>>> On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
>>> 
>>> -1 to (1) and (2).
>>> 
>>> I think merge commits are appropriate in certain circumstances, so I don't
>>> think we should make a blanket restriction. In fact I think our release
>>> process involves some merges.
>>> 
>>> I think setting standards on what is reasonable to be an individual commit
>>> will do a lot more to clean up our history than blocking merges. We'd
>>> rather not see commits like "Spotless Apply" in the history, but if
>>> reasonably separate and well written commits come in as part of a merge, I
>>> think that's fine.
>>> 
>>> -Dan
>>> 
 On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao  wrote:
 
 +1
 
> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols  wrote:
> 
> I’d like to advance this topic from an informal request/discussion to a
> discussion of a concrete proposal.
> 
> To recap, it sounds like there is general agreement that commit history
 on
> develop should be linear (no merge commits), and that the biggest
 obstacle
> to this is that the PR merge button in GitHub creates a merge commit by
> default.
> 
> I propose the following changes:
> 1) Change GitHub settings to remove the ability to create a merge commit.
> This will make Squash & Merge the default.
> 
> 2) Change GitHub settings to require linear history on develop.  This
> prevents merge commits via command-line (not recommended, but wiki still
> has instructions for merging PRs this way).
> 
> 3) Update the PR template to change the text "Is your initial
 contribution
> a single, squashed commit” to “Is your initial contribution squashed for
> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
> ‘fix’ commit)"
> 
> For clarity, I am proposing no-change in the following areas:
> i) Leave Rebase & Merge as an option for PRs that have been structured to
> benefit from it (this can make it easier in a bisect to see whether the
> refactoring or the “fix” broke something).
> ii) Leave existing wording in the wiki as-is [stating that squashing is
> preferred].
> 
> 
> Please comment via this email thread.
> -Owen
> 
> 
> 
>> On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:
>> 
>> I think it's already resolved Udo ;)
>> 
>> Here's the problem, if I fixup a dunit test by removing all uses of
> "this."
>> and I rename the dunit test, then git doesn't remember that the file
 is a
>> rename -- it forever afterwards interprets it as a new file that I
> created
>> if I touch more than 50% of the lines (which "this." can easily do). If
> we
>> squash two commits: the rename and the cleanup of that dunit test --
 then
>> we effectively lose the history of that file and it shows that I
 created
> a
>> new file.
>> 
>> Also for the record, I've been working on Geode since the beginning
 and I
>> was never made aware of this change in our process. I never voted on
 it.
>> I'm not a big fan of changing various details in our process every
 single
>> week. It's very easy to miss these discussions unless someone points it
> out
>> to me.
>> 
>> On Mon, Dec 16, 2019 at 10:34 AM Udo Kohlmeyer 
>> wrote:
>> 
>>> I'm not sure what this discussion is about... WE

Re: [DISCUSS] Proposal to require linear commit history on develop

[Ju@N]

+1

On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:

> Hi Bruce, this proposal will not waste a single second of your time.  It
> just prevents people from accidentally pressing a button that we have
> already agreed should never be pressed, but because we never configured our
> GitHub to match our stated policy, is currently the default.
>
> However, it will save a lot of time and frustration for anyone needing to
> bisect failures, revert, or cherry-pick changes, which has merit even if
> you personally never do any of those three things.
>
> Please start a separate thread if you would like to revisit the community
> decision to require passing PR checks.
>
> > On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 
> wrote:
> >
> > I agree with Jake.  I would go further by saying that I see very little
> merit in this proposal.  I think we're getting more and more bureaucratic
> in our process and that it stifles productivity.  I was recently forced to
> spend three days fixing tests in which I had changed an import statement
> before they would pass stress testing.  I'm glad the tests now pass
> reliably but I was very frustrated by the process.
> >
> > On 12/19/19 4:49 PM, Jacob Barrett wrote:
> >> I’m in agreement with Dan. Changes to the infrastructure to flat out
> prevent things that should be self policing is annoying. This PR review
> lock we have had already cost us valuable time waiting for PR pipelines to
> pass that have no relevance to the commit, like CI work: I’d hat to see yet
> another process enforced that Kees us from getting work done when necessary.
> >>
> >> -Jake
> >>
> >>
> >>> On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> >>>
> >>> -1 to (1) and (2).
> >>>
> >>> I think merge commits are appropriate in certain circumstances, so I
> don't
> >>> think we should make a blanket restriction. In fact I think our release
> >>> process involves some merges.
> >>>
> >>> I think setting standards on what is reasonable to be an individual
> commit
> >>> will do a lot more to clean up our history than blocking merges. We'd
> >>> rather not see commits like "Spotless Apply" in the history, but if
> >>> reasonably separate and well written commits come in as part of a
> merge, I
> >>> think that's fine.
> >>>
> >>> -Dan
> >>>
>  On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
> wrote:
> 
>  +1
> 
> > On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 
> wrote:
> >
> > I’d like to advance this topic from an informal request/discussion
> to a
> > discussion of a concrete proposal.
> >
> > To recap, it sounds like there is general agreement that commit
> history
>  on
> > develop should be linear (no merge commits), and that the biggest
>  obstacle
> > to this is that the PR merge button in GitHub creates a merge commit
> by
> > default.
> >
> > I propose the following changes:
> > 1) Change GitHub settings to remove the ability to create a merge
> commit.
> > This will make Squash & Merge the default.
> >
> > 2) Change GitHub settings to require linear history on develop.  This
> > prevents merge commits via command-line (not recommended, but wiki
> still
> > has instructions for merging PRs this way).
> >
> > 3) Update the PR template to change the text "Is your initial
>  contribution
> > a single, squashed commit” to “Is your initial contribution squashed
> for
> > ease of review (e.g. a single commit, or a ‘refactoring’ commit plus
> a
> > ‘fix’ commit)"
> >
> > For clarity, I am proposing no-change in the following areas:
> > i) Leave Rebase & Merge as an option for PRs that have been
> structured to
> > benefit from it (this can make it easier in a bisect to see whether
> the
> > refactoring or the “fix” broke something).
> > ii) Leave existing wording in the wiki as-is [stating that squashing
> is
> > preferred].
> >
> >
> > Please comment via this email thread.
> > -Owen
> >
> >
> >
> >> On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:
> >>
> >> I think it's already resolved Udo ;)
> >>
> >> Here's the problem, if I fixup a dunit test by removing all uses of
> > "this."
> >> and I rename the dunit test, then git doesn't remember that the file
>  is a
> >> rename -- it forever afterwards interprets it as a new file that I
> > created
> >> if I touch more than 50% of the lines (which "this." can easily
> do). If
> > we
> >> squash two commits: the rename and the cleanup of that dunit test --
>  then
> >> we effectively lose the history of that file and it shows that I
>  created
> > a
> >> new file.
> >>
> >> Also for the record, I've been working on Geode since the beginning
>  and I
> >> was never made aware of this change in our process. I never voted on
>  it.
> >> I'm not a big fan of changing various details in our process every
> >>>

Re: [DISCUSS] Proposal to require linear commit history on develop

[Aaron Lindsey]

+1 to (1) and (3)

I’m on board with (1). I’m hesitant about agreeing to (2) because it seems 
harder to “accidentally” do a merge commit via the command line, and I don’t 
want to add unnecessary restrictions. (3) has needed to be done for some time 
now, so I’m happy to see a proposal to change that.

In which case would an explicit merge commit be preferred/required to a “rebase 
and merge”? In my experience working on Geode, I’ve never needed to create an 
explicit merge commit. I have, however, seen people do it by accident. As a 
reminder, “rebase and merge" sill allows you to keep all of the individual 
commits from your PR.

- Aaron

> On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> 
> +1
> 
> On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:
> 
>> Hi Bruce, this proposal will not waste a single second of your time.  It
>> just prevents people from accidentally pressing a button that we have
>> already agreed should never be pressed, but because we never configured our
>> GitHub to match our stated policy, is currently the default.
>> 
>> However, it will save a lot of time and frustration for anyone needing to
>> bisect failures, revert, or cherry-pick changes, which has merit even if
>> you personally never do any of those three things.
>> 
>> Please start a separate thread if you would like to revisit the community
>> decision to require passing PR checks.
>> 
>>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> I agree with Jake.  I would go further by saying that I see very little
>> merit in this proposal.  I think we're getting more and more bureaucratic
>> in our process and that it stifles productivity.  I was recently forced to
>> spend three days fixing tests in which I had changed an import statement
>> before they would pass stress testing.  I'm glad the tests now pass
>> reliably but I was very frustrated by the process.
>>> 
>>> On 12/19/19 4:49 PM, Jacob Barrett wrote:
 I’m in agreement with Dan. Changes to the infrastructure to flat out
>> prevent things that should be self policing is annoying. This PR review
>> lock we have had already cost us valuable time waiting for PR pipelines to
>> pass that have no relevance to the commit, like CI work: I’d hat to see yet
>> another process enforced that Kees us from getting work done when necessary.
 
 -Jake
 
 
> On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> 
> -1 to (1) and (2).
> 
> I think merge commits are appropriate in certain circumstances, so I
>> don't
> think we should make a blanket restriction. In fact I think our release
> process involves some merges.
> 
> I think setting standards on what is reasonable to be an individual
>> commit
> will do a lot more to clean up our history than blocking merges. We'd
> rather not see commits like "Spotless Apply" in the history, but if
> reasonably separate and well written commits come in as part of a
>> merge, I
> think that's fine.
> 
> -Dan
> 
>> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
>> wrote:
>> 
>> +1
>> 
>>> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 
>> wrote:
>>> 
>>> I’d like to advance this topic from an informal request/discussion
>> to a
>>> discussion of a concrete proposal.
>>> 
>>> To recap, it sounds like there is general agreement that commit
>> history
>> on
>>> develop should be linear (no merge commits), and that the biggest
>> obstacle
>>> to this is that the PR merge button in GitHub creates a merge commit
>> by
>>> default.
>>> 
>>> I propose the following changes:
>>> 1) Change GitHub settings to remove the ability to create a merge
>> commit.
>>> This will make Squash & Merge the default.
>>> 
>>> 2) Change GitHub settings to require linear history on develop.  This
>>> prevents merge commits via command-line (not recommended, but wiki
>> still
>>> has instructions for merging PRs this way).
>>> 
>>> 3) Update the PR template to change the text "Is your initial
>> contribution
>>> a single, squashed commit” to “Is your initial contribution squashed
>> for
>>> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus
>> a
>>> ‘fix’ commit)"
>>> 
>>> For clarity, I am proposing no-change in the following areas:
>>> i) Leave Rebase & Merge as an option for PRs that have been
>> structured to
>>> benefit from it (this can make it easier in a bisect to see whether
>> the
>>> refactoring or the “fix” broke something).
>>> ii) Leave existing wording in the wiki as-is [stating that squashing
>> is
>>> preferred].
>>> 
>>> 
>>> Please comment via this email thread.
>>> -Owen
>>> 
>>> 
>>> 
 On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:
 
 I think it's already resolved Udo ;)
 
 Here's the problem, if I fixup a dunit test by removing all uses of

Re: [DISCUSS] Proposal to require linear commit history on develop

[Owen Nichols]

Based on the feedback so far, I will amend the proposal to drop item 2).  
Therefore, the current ability to create merge commits using command-line git 
will remain available.

The proposal as amended is now:
> Change GitHub settings to make "Squash and merge" the default
> (by removing “Create a merge commit” option).
> 
> Update the PR template to change the text "Is your initial contribution
> a single, squashed commit” to “Is your initial contribution squashed for
> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
> ‘fix’ commit)"


As Naba suggested, we can try it, and if we don’t like it, it’s simple to 
revert.

I’ve create a PR for the proposed change here: 
https://github.com/apache/geode/pull/4513

Please use the PR to vote for against this proposal.  It will not be merged 
before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)

> On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> 
> +1
> 
> On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:
> 
>> Hi Bruce, this proposal will not waste a single second of your time.  It
>> just prevents people from accidentally pressing a button that we have
>> already agreed should never be pressed, but because we never configured our
>> GitHub to match our stated policy, is currently the default.
>> 
>> However, it will save a lot of time and frustration for anyone needing to
>> bisect failures, revert, or cherry-pick changes, which has merit even if
>> you personally never do any of those three things.
>> 
>> Please start a separate thread if you would like to revisit the community
>> decision to require passing PR checks.
>> 
>>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> I agree with Jake.  I would go further by saying that I see very little
>> merit in this proposal.  I think we're getting more and more bureaucratic
>> in our process and that it stifles productivity.  I was recently forced to
>> spend three days fixing tests in which I had changed an import statement
>> before they would pass stress testing.  I'm glad the tests now pass
>> reliably but I was very frustrated by the process.
>>> 
>>> On 12/19/19 4:49 PM, Jacob Barrett wrote:
 I’m in agreement with Dan. Changes to the infrastructure to flat out
>> prevent things that should be self policing is annoying. This PR review
>> lock we have had already cost us valuable time waiting for PR pipelines to
>> pass that have no relevance to the commit, like CI work: I’d hat to see yet
>> another process enforced that Kees us from getting work done when necessary.
 
 -Jake
 
 
> On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> 
> -1 to (1) and (2).
> 
> I think merge commits are appropriate in certain circumstances, so I
>> don't
> think we should make a blanket restriction. In fact I think our release
> process involves some merges.
> 
> I think setting standards on what is reasonable to be an individual
>> commit
> will do a lot more to clean up our history than blocking merges. We'd
> rather not see commits like "Spotless Apply" in the history, but if
> reasonably separate and well written commits come in as part of a
>> merge, I
> think that's fine.
> 
> -Dan
> 
>> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
>> wrote:
>> 
>> +1
>> 
>>> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 
>> wrote:
>>> 
>>> I’d like to advance this topic from an informal request/discussion
>> to a
>>> discussion of a concrete proposal.
>>> 
>>> To recap, it sounds like there is general agreement that commit
>> history
>> on
>>> develop should be linear (no merge commits), and that the biggest
>> obstacle
>>> to this is that the PR merge button in GitHub creates a merge commit
>> by
>>> default.
>>> 
>>> I propose the following changes:
>>> 1) Change GitHub settings to remove the ability to create a merge
>> commit.
>>> This will make Squash & Merge the default.
>>> 
>>> 2) Change GitHub settings to require linear history on develop.  This
>>> prevents merge commits via command-line (not recommended, but wiki
>> still
>>> has instructions for merging PRs this way).
>>> 
>>> 3) Update the PR template to change the text "Is your initial
>> contribution
>>> a single, squashed commit” to “Is your initial contribution squashed
>> for
>>> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus
>> a
>>> ‘fix’ commit)"
>>> 
>>> For clarity, I am proposing no-change in the following areas:
>>> i) Leave Rebase & Merge as an option for PRs that have been
>> structured to
>>> benefit from it (this can make it easier in a bisect to see whether
>> the
>>> refactoring or the “fix” broke something).
>>> ii) Leave existing wording in the wiki as-is [stating that squashing
>> is
>>> preferred].
>>> 
>>> 
>>> Please comment via this email thread.
>>> 

Re: [DISCUSS] Proposal to require linear commit history on develop

[Nabarun Nag]

+1

On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols  wrote:

> Based on the feedback so far, I will amend the proposal to drop item 2).
> Therefore, the current ability to create merge commits using command-line
> git will remain available.
>
> The proposal as amended is now:
> > Change GitHub settings to make "Squash and merge" the default
> > (by removing “Create a merge commit” option).
> >
> > Update the PR template to change the text "Is your initial contribution
> > a single, squashed commit” to “Is your initial contribution squashed for
> > ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
> > ‘fix’ commit)"
>
>
> As Naba suggested, we can try it, and if we don’t like it, it’s simple to
> revert.
>
> I’ve create a PR for the proposed change here:
> https://github.com/apache/geode/pull/4513
>
> Please use the PR to vote for against this proposal.  It will not be
> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)
>
> > On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> >
> > +1
> >
> > On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:
> >
> >> Hi Bruce, this proposal will not waste a single second of your time.  It
> >> just prevents people from accidentally pressing a button that we have
> >> already agreed should never be pressed, but because we never configured
> our
> >> GitHub to match our stated policy, is currently the default.
> >>
> >> However, it will save a lot of time and frustration for anyone needing
> to
> >> bisect failures, revert, or cherry-pick changes, which has merit even if
> >> you personally never do any of those three things.
> >>
> >> Please start a separate thread if you would like to revisit the
> community
> >> decision to require passing PR checks.
> >>
> >>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 
> >> wrote:
> >>>
> >>> I agree with Jake.  I would go further by saying that I see very little
> >> merit in this proposal.  I think we're getting more and more
> bureaucratic
> >> in our process and that it stifles productivity.  I was recently forced
> to
> >> spend three days fixing tests in which I had changed an import statement
> >> before they would pass stress testing.  I'm glad the tests now pass
> >> reliably but I was very frustrated by the process.
> >>>
> >>> On 12/19/19 4:49 PM, Jacob Barrett wrote:
>  I’m in agreement with Dan. Changes to the infrastructure to flat out
> >> prevent things that should be self policing is annoying. This PR review
> >> lock we have had already cost us valuable time waiting for PR pipelines
> to
> >> pass that have no relevance to the commit, like CI work: I’d hat to see
> yet
> >> another process enforced that Kees us from getting work done when
> necessary.
> 
>  -Jake
> 
> 
> > On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> >
> > -1 to (1) and (2).
> >
> > I think merge commits are appropriate in certain circumstances, so I
> >> don't
> > think we should make a blanket restriction. In fact I think our
> release
> > process involves some merges.
> >
> > I think setting standards on what is reasonable to be an individual
> >> commit
> > will do a lot more to clean up our history than blocking merges. We'd
> > rather not see commits like "Spotless Apply" in the history, but if
> > reasonably separate and well written commits come in as part of a
> >> merge, I
> > think that's fine.
> >
> > -Dan
> >
> >> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
> >> wrote:
> >>
> >> +1
> >>
> >>> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 
> >> wrote:
> >>>
> >>> I’d like to advance this topic from an informal request/discussion
> >> to a
> >>> discussion of a concrete proposal.
> >>>
> >>> To recap, it sounds like there is general agreement that commit
> >> history
> >> on
> >>> develop should be linear (no merge commits), and that the biggest
> >> obstacle
> >>> to this is that the PR merge button in GitHub creates a merge
> commit
> >> by
> >>> default.
> >>>
> >>> I propose the following changes:
> >>> 1) Change GitHub settings to remove the ability to create a merge
> >> commit.
> >>> This will make Squash & Merge the default.
> >>>
> >>> 2) Change GitHub settings to require linear history on develop.
> This
> >>> prevents merge commits via command-line (not recommended, but wiki
> >> still
> >>> has instructions for merging PRs this way).
> >>>
> >>> 3) Update the PR template to change the text "Is your initial
> >> contribution
> >>> a single, squashed commit” to “Is your initial contribution
> squashed
> >> for
> >>> ease of review (e.g. a single commit, or a ‘refactoring’ commit
> plus
> >> a
> >>> ‘fix’ commit)"
> >>>
> >>> For clarity, I am proposing no-change in the following areas:
> >>> i) Leave Rebase & Merge as an option for PRs that have been
> >> structured to
> >>> benefit f

Re: [DISCUSS] Proposal to require linear commit history on develop

[Bruce Schuchardt]

I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread 
and a request to "vote" using a PR.  This all seems out of order to me.  
Our votes are supposed to be on the email list, aren't they? and I 
haven't seen a VOTE request.


On 12/20/19 9:33 AM, Nabarun Nag wrote:

+1

On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols  wrote:


Based on the feedback so far, I will amend the proposal to drop item 2).
Therefore, the current ability to create merge commits using command-line
git will remain available.

The proposal as amended is now:

Change GitHub settings to make "Squash and merge" the default
(by removing “Create a merge commit” option).

Update the PR template to change the text "Is your initial contribution
a single, squashed commit” to “Is your initial contribution squashed for
ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
‘fix’ commit)"


As Naba suggested, we can try it, and if we don’t like it, it’s simple to
revert.

I’ve create a PR for the proposed change here:
https://github.com/apache/geode/pull/4513

Please use the PR to vote for against this proposal.  It will not be
merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)


On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:

+1

On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:


Hi Bruce, this proposal will not waste a single second of your time.  It
just prevents people from accidentally pressing a button that we have
already agreed should never be pressed, but because we never configured

our

GitHub to match our stated policy, is currently the default.

However, it will save a lot of time and frustration for anyone needing

to

bisect failures, revert, or cherry-pick changes, which has merit even if
you personally never do any of those three things.

Please start a separate thread if you would like to revisit the

community

decision to require passing PR checks.


On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 

wrote:

I agree with Jake.  I would go further by saying that I see very little

merit in this proposal.  I think we're getting more and more

bureaucratic

in our process and that it stifles productivity.  I was recently forced

to

spend three days fixing tests in which I had changed an import statement
before they would pass stress testing.  I'm glad the tests now pass
reliably but I was very frustrated by the process.

On 12/19/19 4:49 PM, Jacob Barrett wrote:

I’m in agreement with Dan. Changes to the infrastructure to flat out

prevent things that should be self policing is annoying. This PR review
lock we have had already cost us valuable time waiting for PR pipelines

to

pass that have no relevance to the commit, like CI work: I’d hat to see

yet

another process enforced that Kees us from getting work done when

necessary.

-Jake



On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:

-1 to (1) and (2).

I think merge commits are appropriate in certain circumstances, so I

don't

think we should make a blanket restriction. In fact I think our

release

process involves some merges.

I think setting standards on what is reasonable to be an individual

commit

will do a lot more to clean up our history than blocking merges. We'd
rather not see commits like "Spotless Apply" in the history, but if
reasonably separate and well written commits come in as part of a

merge, I

think that's fine.

-Dan


On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 

wrote:

+1


On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 

wrote:

I’d like to advance this topic from an informal request/discussion

to a

discussion of a concrete proposal.

To recap, it sounds like there is general agreement that commit

history

on

develop should be linear (no merge commits), and that the biggest

obstacle

to this is that the PR merge button in GitHub creates a merge

commit

by

default.

I propose the following changes:
1) Change GitHub settings to remove the ability to create a merge

commit.

This will make Squash & Merge the default.

2) Change GitHub settings to require linear history on develop.

This

prevents merge commits via command-line (not recommended, but wiki

still

has instructions for merging PRs this way).

3) Update the PR template to change the text "Is your initial

contribution

a single, squashed commit” to “Is your initial contribution

squashed

for

ease of review (e.g. a single commit, or a ‘refactoring’ commit

plus

a

‘fix’ commit)"

For clarity, I am proposing no-change in the following areas:
i) Leave Rebase & Merge as an option for PRs that have been

structured to

benefit from it (this can make it easier in a bisect to see whether

the

refactoring or the “fix” broke something).
ii) Leave existing wording in the wiki as-is [stating that

squashing

is

preferred].


Please comment via this email thread.
-Owen




On Dec 16, 2019, at 10:49 AM, Kirk Lund  wrote:

I think it's already resolved Udo ;)

Here's the problem, if I fixup a dunit test by removing all uses

of

"this."

and I ren

Re: [DISCUSS] Proposal to require linear commit history on develop

[Owen Nichols]

The proposed action manifests as a commit to the Geode git repository, so a PR 
is an acceptable vehicle for voting in this case.

> On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt  wrote:
> 
> I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread and a 
> request to "vote" using a PR.  This all seems out of order to me.  Our votes 
> are supposed to be on the email list, aren't they? and I haven't seen a VOTE 
> request.
> 
> On 12/20/19 9:33 AM, Nabarun Nag wrote:
>> +1
>> 
>> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols  wrote:
>> 
>>> Based on the feedback so far, I will amend the proposal to drop item 2).
>>> Therefore, the current ability to create merge commits using command-line
>>> git will remain available.
>>> 
>>> The proposal as amended is now:
 Change GitHub settings to make "Squash and merge" the default
 (by removing “Create a merge commit” option).
 
 Update the PR template to change the text "Is your initial contribution
 a single, squashed commit” to “Is your initial contribution squashed for
 ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
 ‘fix’ commit)"
>>> 
>>> As Naba suggested, we can try it, and if we don’t like it, it’s simple to
>>> revert.
>>> 
>>> I’ve create a PR for the proposed change here:
>>> https://github.com/apache/geode/pull/4513
>>> 
>>> Please use the PR to vote for against this proposal.  It will not be
>>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)
>>> 
 On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
 
 +1
 
 On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:
 
> Hi Bruce, this proposal will not waste a single second of your time.  It
> just prevents people from accidentally pressing a button that we have
> already agreed should never be pressed, but because we never configured
>>> our
> GitHub to match our stated policy, is currently the default.
> 
> However, it will save a lot of time and frustration for anyone needing
>>> to
> bisect failures, revert, or cherry-pick changes, which has merit even if
> you personally never do any of those three things.
> 
> Please start a separate thread if you would like to revisit the
>>> community
> decision to require passing PR checks.
> 
>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 
> wrote:
>> I agree with Jake.  I would go further by saying that I see very little
> merit in this proposal.  I think we're getting more and more
>>> bureaucratic
> in our process and that it stifles productivity.  I was recently forced
>>> to
> spend three days fixing tests in which I had changed an import statement
> before they would pass stress testing.  I'm glad the tests now pass
> reliably but I was very frustrated by the process.
>> On 12/19/19 4:49 PM, Jacob Barrett wrote:
>>> I’m in agreement with Dan. Changes to the infrastructure to flat out
> prevent things that should be self policing is annoying. This PR review
> lock we have had already cost us valuable time waiting for PR pipelines
>>> to
> pass that have no relevance to the commit, like CI work: I’d hat to see
>>> yet
> another process enforced that Kees us from getting work done when
>>> necessary.
>>> -Jake
>>> 
>>> 
 On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
 
 -1 to (1) and (2).
 
 I think merge commits are appropriate in certain circumstances, so I
> don't
 think we should make a blanket restriction. In fact I think our
>>> release
 process involves some merges.
 
 I think setting standards on what is reasonable to be an individual
> commit
 will do a lot more to clean up our history than blocking merges. We'd
 rather not see commits like "Spotless Apply" in the history, but if
 reasonably separate and well written commits come in as part of a
> merge, I
 think that's fine.
 
 -Dan
 
> On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
> wrote:
> +1
> 
>> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 
> wrote:
>> I’d like to advance this topic from an informal request/discussion
> to a
>> discussion of a concrete proposal.
>> 
>> To recap, it sounds like there is general agreement that commit
> history
> on
>> develop should be linear (no merge commits), and that the biggest
> obstacle
>> to this is that the PR merge button in GitHub creates a merge
>>> commit
> by
>> default.
>> 
>> I propose the following changes:
>> 1) Change GitHub settings to remove the ability to create a merge
> commit.
>> This will make Squash & Merge the default.
>> 
>> 2) Change GitHub settings to require linear history on develop.
>>> This
>> prevents

Re: [DISCUSS] Proposal to require linear commit history on develop

[Bruce Schuchardt]

well, I always use squash-and-merge so I'm not against the primary 
motivation for this thread.


On 12/20/19 10:07 AM, Owen Nichols wrote:

The proposed action manifests as a commit to the Geode git repository, so a PR 
is an acceptable vehicle for voting in this case.


On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt  wrote:

I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread and a request to 
"vote" using a PR.  This all seems out of order to me.  Our votes are supposed to be on 
the email list, aren't they? and I haven't seen a VOTE request.

On 12/20/19 9:33 AM, Nabarun Nag wrote:

+1

On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols  wrote:


Based on the feedback so far, I will amend the proposal to drop item 2).
Therefore, the current ability to create merge commits using command-line
git will remain available.

The proposal as amended is now:

Change GitHub settings to make "Squash and merge" the default
(by removing “Create a merge commit” option).

Update the PR template to change the text "Is your initial contribution
a single, squashed commit” to “Is your initial contribution squashed for
ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
‘fix’ commit)"

As Naba suggested, we can try it, and if we don’t like it, it’s simple to
revert.

I’ve create a PR for the proposed change here:
https://github.com/apache/geode/pull/4513

Please use the PR to vote for against this proposal.  It will not be
merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)


On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:

+1

On Fri 20 Dec 2019 at 16:18, Owen Nichols  wrote:


Hi Bruce, this proposal will not waste a single second of your time.  It
just prevents people from accidentally pressing a button that we have
already agreed should never be pressed, but because we never configured

our

GitHub to match our stated policy, is currently the default.

However, it will save a lot of time and frustration for anyone needing

to

bisect failures, revert, or cherry-pick changes, which has merit even if
you personally never do any of those three things.

Please start a separate thread if you would like to revisit the

community

decision to require passing PR checks.


On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt 

wrote:

I agree with Jake.  I would go further by saying that I see very little

merit in this proposal.  I think we're getting more and more

bureaucratic

in our process and that it stifles productivity.  I was recently forced

to

spend three days fixing tests in which I had changed an import statement
before they would pass stress testing.  I'm glad the tests now pass
reliably but I was very frustrated by the process.

On 12/19/19 4:49 PM, Jacob Barrett wrote:

I’m in agreement with Dan. Changes to the infrastructure to flat out

prevent things that should be self policing is annoying. This PR review
lock we have had already cost us valuable time waiting for PR pipelines

to

pass that have no relevance to the commit, like CI work: I’d hat to see

yet

another process enforced that Kees us from getting work done when

necessary.

-Jake



On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:

-1 to (1) and (2).

I think merge commits are appropriate in certain circumstances, so I

don't

think we should make a blanket restriction. In fact I think our

release

process involves some merges.

I think setting standards on what is reasonable to be an individual

commit

will do a lot more to clean up our history than blocking merges. We'd
rather not see commits like "Spotless Apply" in the history, but if
reasonably separate and well written commits come in as part of a

merge, I

think that's fine.

-Dan


On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 

wrote:

+1


On Thu, Dec 19, 2019, 4:05 PM Owen Nichols 

wrote:

I’d like to advance this topic from an informal request/discussion

to a

discussion of a concrete proposal.

To recap, it sounds like there is general agreement that commit

history

on

develop should be linear (no merge commits), and that the biggest

obstacle

to this is that the PR merge button in GitHub creates a merge

commit

by

default.

I propose the following changes:
1) Change GitHub settings to remove the ability to create a merge

commit.

This will make Squash & Merge the default.

2) Change GitHub settings to require linear history on develop.

This

prevents merge commits via command-line (not recommended, but wiki

still

has instructions for merging PRs this way).

3) Update the PR template to change the text "Is your initial

contribution

a single, squashed commit” to “Is your initial contribution

squashed

for

ease of review (e.g. a single commit, or a ‘refactoring’ commit

plus

a

‘fix’ commit)"

For clarity, I am proposing no-change in the following areas:
i) Leave Rebase & Merge as an option for PRs that have been

structured to

benefit from it (this can make it easier in a bisect to see whether

the

refactoring or the “fix” broke

Re: [DISCUSS] Proposal to require linear commit history on develop

[Ernest Burghardt]

I'm a proponent of using squash-and-merge, and once a person has chosen
this option once it comes up by default afterwards...

Owen,  I don't think you have consensus to put forth this PR, there are -1s
above... (early voting)

maybe we'll be better off socializing the norm of squash-and-merge and
gaining a natural consensus that way...

On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols  wrote:

> The proposed action manifests as a commit to the Geode git repository, so
> a PR is an acceptable vehicle for voting in this case.
>
> > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt 
> wrote:
> >
> > I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread
> and a request to "vote" using a PR.  This all seems out of order to me.
> Our votes are supposed to be on the email list, aren't they? and I haven't
> seen a VOTE request.
> >
> > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> >> +1
> >>
> >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
> wrote:
> >>
> >>> Based on the feedback so far, I will amend the proposal to drop item
> 2).
> >>> Therefore, the current ability to create merge commits using
> command-line
> >>> git will remain available.
> >>>
> >>> The proposal as amended is now:
>  Change GitHub settings to make "Squash and merge" the default
>  (by removing “Create a merge commit” option).
> 
>  Update the PR template to change the text "Is your initial
> contribution
>  a single, squashed commit” to “Is your initial contribution squashed
> for
>  ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
>  ‘fix’ commit)"
> >>>
> >>> As Naba suggested, we can try it, and if we don’t like it, it’s simple
> to
> >>> revert.
> >>>
> >>> I’ve create a PR for the proposed change here:
> >>> https://github.com/apache/geode/pull/4513
> >>>
> >>> Please use the PR to vote for against this proposal.  It will not be
> >>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)
> >>>
>  On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> 
>  +1
> 
>  On Fri 20 Dec 2019 at 16:18, Owen Nichols 
> wrote:
> 
> > Hi Bruce, this proposal will not waste a single second of your
> time.  It
> > just prevents people from accidentally pressing a button that we have
> > already agreed should never be pressed, but because we never
> configured
> >>> our
> > GitHub to match our stated policy, is currently the default.
> >
> > However, it will save a lot of time and frustration for anyone
> needing
> >>> to
> > bisect failures, revert, or cherry-pick changes, which has merit
> even if
> > you personally never do any of those three things.
> >
> > Please start a separate thread if you would like to revisit the
> >>> community
> > decision to require passing PR checks.
> >
> >> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt <
> bschucha...@pivotal.io>
> > wrote:
> >> I agree with Jake.  I would go further by saying that I see very
> little
> > merit in this proposal.  I think we're getting more and more
> >>> bureaucratic
> > in our process and that it stifles productivity.  I was recently
> forced
> >>> to
> > spend three days fixing tests in which I had changed an import
> statement
> > before they would pass stress testing.  I'm glad the tests now pass
> > reliably but I was very frustrated by the process.
> >> On 12/19/19 4:49 PM, Jacob Barrett wrote:
> >>> I’m in agreement with Dan. Changes to the infrastructure to flat
> out
> > prevent things that should be self policing is annoying. This PR
> review
> > lock we have had already cost us valuable time waiting for PR
> pipelines
> >>> to
> > pass that have no relevance to the commit, like CI work: I’d hat to
> see
> >>> yet
> > another process enforced that Kees us from getting work done when
> >>> necessary.
> >>> -Jake
> >>>
> >>>
>  On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
> 
>  -1 to (1) and (2).
> 
>  I think merge commits are appropriate in certain circumstances,
> so I
> > don't
>  think we should make a blanket restriction. In fact I think our
> >>> release
>  process involves some merges.
> 
>  I think setting standards on what is reasonable to be an
> individual
> > commit
>  will do a lot more to clean up our history than blocking merges.
> We'd
>  rather not see commits like "Spotless Apply" in the history, but
> if
>  reasonably separate and well written commits come in as part of a
> > merge, I
>  think that's fine.
> 
>  -Dan
> 
> > On Thu, Dec 19, 2019 at 4:27 PM Jinmei Liao 
> > wrote:
> > +1
> >
> >> On Thu, Dec 19, 2019, 4:05 PM Owen Nichols  >
> > wrote:
> >> I’d like to advance this topic from an informal
> request/discussion
> > to a
> >> discussion of a conc

Re: [DISCUSS] Proposal to require linear commit history on develop

[Owen Nichols]

The objections in the discuss thread all seemed to hinge on item 2) of the 
original proposal.  With this item removed, perhaps many or all of those people 
would no longer vote -1.

Please feel free to “request changes” on the PR if you still want something 
different -or- to indicate you believe we should not make this change at all.  
The PR will not be merged unless there is unanimous approval by DEC 31.

> On Dec 20, 2019, at 10:18 AM, Ernest Burghardt  wrote:
> 
> I'm a proponent of using squash-and-merge, and once a person has chosen
> this option once it comes up by default afterwards...
> 
> Owen,  I don't think you have consensus to put forth this PR, there are -1s
> above... (early voting)
> 
> maybe we'll be better off socializing the norm of squash-and-merge and
> gaining a natural consensus that way...
> 
> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols  wrote:
> 
>> The proposed action manifests as a commit to the Geode git repository, so
>> a PR is an acceptable vehicle for voting in this case.
>> 
>>> On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt 
>> wrote:
>>> 
>>> I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread
>> and a request to "vote" using a PR.  This all seems out of order to me.
>> Our votes are supposed to be on the email list, aren't they? and I haven't
>> seen a VOTE request.
>>> 
>>> On 12/20/19 9:33 AM, Nabarun Nag wrote:
 +1
 
 On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
>> wrote:
 
> Based on the feedback so far, I will amend the proposal to drop item
>> 2).
> Therefore, the current ability to create merge commits using
>> command-line
> git will remain available.
> 
> The proposal as amended is now:
>> Change GitHub settings to make "Squash and merge" the default
>> (by removing “Create a merge commit” option).
>> 
>> Update the PR template to change the text "Is your initial
>> contribution
>> a single, squashed commit” to “Is your initial contribution squashed
>> for
>> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
>> ‘fix’ commit)"
> 
> As Naba suggested, we can try it, and if we don’t like it, it’s simple
>> to
> revert.
> 
> I’ve create a PR for the proposed change here:
> https://github.com/apache/geode/pull/4513
> 
> Please use the PR to vote for against this proposal.  It will not be
> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)
> 
>> On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
>> 
>> +1
>> 
>> On Fri 20 Dec 2019 at 16:18, Owen Nichols 
>> wrote:
>> 
>>> Hi Bruce, this proposal will not waste a single second of your
>> time.  It
>>> just prevents people from accidentally pressing a button that we have
>>> already agreed should never be pressed, but because we never
>> configured
> our
>>> GitHub to match our stated policy, is currently the default.
>>> 
>>> However, it will save a lot of time and frustration for anyone
>> needing
> to
>>> bisect failures, revert, or cherry-pick changes, which has merit
>> even if
>>> you personally never do any of those three things.
>>> 
>>> Please start a separate thread if you would like to revisit the
> community
>>> decision to require passing PR checks.
>>> 
 On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt <
>> bschucha...@pivotal.io>
>>> wrote:
 I agree with Jake.  I would go further by saying that I see very
>> little
>>> merit in this proposal.  I think we're getting more and more
> bureaucratic
>>> in our process and that it stifles productivity.  I was recently
>> forced
> to
>>> spend three days fixing tests in which I had changed an import
>> statement
>>> before they would pass stress testing.  I'm glad the tests now pass
>>> reliably but I was very frustrated by the process.
 On 12/19/19 4:49 PM, Jacob Barrett wrote:
> I’m in agreement with Dan. Changes to the infrastructure to flat
>> out
>>> prevent things that should be self policing is annoying. This PR
>> review
>>> lock we have had already cost us valuable time waiting for PR
>> pipelines
> to
>>> pass that have no relevance to the commit, like CI work: I’d hat to
>> see
> yet
>>> another process enforced that Kees us from getting work done when
> necessary.
> -Jake
> 
> 
>> On Dec 19, 2019, at 4:43 PM, Dan Smith  wrote:
>> 
>> -1 to (1) and (2).
>> 
>> I think merge commits are appropriate in certain circumstances,
>> so I
>>> don't
>> think we should make a blanket restriction. In fact I think our
> release
>> process involves some merges.
>> 
>> I think setting standards on what is reasonable to be an
>> individual
>>> commit
>> will do a lot more to clean up our history than blocking merges.
>> We'

Re: [DISCUSS] Proposal to require linear commit history on develop

[Jinmei Liao]

Merge commit is the new contributor's default setting. When we are merging
new contributor's PR, since we are so used to THINKING "squash-and-merge"
is the default, we forgot to check what the button really says when we are
merging other people's PR.

On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt 
wrote:

> I'm a proponent of using squash-and-merge, and once a person has chosen
> this option once it comes up by default afterwards...
>
> Owen,  I don't think you have consensus to put forth this PR, there are -1s
> above... (early voting)
>
> maybe we'll be better off socializing the norm of squash-and-merge and
> gaining a natural consensus that way...
>
> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols  wrote:
>
> > The proposed action manifests as a commit to the Geode git repository, so
> > a PR is an acceptable vehicle for voting in this case.
> >
> > > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt 
> > wrote:
> > >
> > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread
> > and a request to "vote" using a PR.  This all seems out of order to me.
> > Our votes are supposed to be on the email list, aren't they? and I
> haven't
> > seen a VOTE request.
> > >
> > > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> > >> +1
> > >>
> > >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
> > wrote:
> > >>
> > >>> Based on the feedback so far, I will amend the proposal to drop item
> > 2).
> > >>> Therefore, the current ability to create merge commits using
> > command-line
> > >>> git will remain available.
> > >>>
> > >>> The proposal as amended is now:
> >  Change GitHub settings to make "Squash and merge" the default
> >  (by removing “Create a merge commit” option).
> > 
> >  Update the PR template to change the text "Is your initial
> > contribution
> >  a single, squashed commit” to “Is your initial contribution squashed
> > for
> >  ease of review (e.g. a single commit, or a ‘refactoring’ commit
> plus a
> >  ‘fix’ commit)"
> > >>>
> > >>> As Naba suggested, we can try it, and if we don’t like it, it’s
> simple
> > to
> > >>> revert.
> > >>>
> > >>> I’ve create a PR for the proposed change here:
> > >>> https://github.com/apache/geode/pull/4513
> > >>>
> > >>> Please use the PR to vote for against this proposal.  It will not be
> > >>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)
> > >>>
> >  On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> > 
> >  +1
> > 
> >  On Fri 20 Dec 2019 at 16:18, Owen Nichols 
> > wrote:
> > 
> > > Hi Bruce, this proposal will not waste a single second of your
> > time.  It
> > > just prevents people from accidentally pressing a button that we
> have
> > > already agreed should never be pressed, but because we never
> > configured
> > >>> our
> > > GitHub to match our stated policy, is currently the default.
> > >
> > > However, it will save a lot of time and frustration for anyone
> > needing
> > >>> to
> > > bisect failures, revert, or cherry-pick changes, which has merit
> > even if
> > > you personally never do any of those three things.
> > >
> > > Please start a separate thread if you would like to revisit the
> > >>> community
> > > decision to require passing PR checks.
> > >
> > >> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt <
> > bschucha...@pivotal.io>
> > > wrote:
> > >> I agree with Jake.  I would go further by saying that I see very
> > little
> > > merit in this proposal.  I think we're getting more and more
> > >>> bureaucratic
> > > in our process and that it stifles productivity.  I was recently
> > forced
> > >>> to
> > > spend three days fixing tests in which I had changed an import
> > statement
> > > before they would pass stress testing.  I'm glad the tests now pass
> > > reliably but I was very frustrated by the process.
> > >> On 12/19/19 4:49 PM, Jacob Barrett wrote:
> > >>> I’m in agreement with Dan. Changes to the infrastructure to flat
> > out
> > > prevent things that should be self policing is annoying. This PR
> > review
> > > lock we have had already cost us valuable time waiting for PR
> > pipelines
> > >>> to
> > > pass that have no relevance to the commit, like CI work: I’d hat to
> > see
> > >>> yet
> > > another process enforced that Kees us from getting work done when
> > >>> necessary.
> > >>> -Jake
> > >>>
> > >>>
> >  On Dec 19, 2019, at 4:43 PM, Dan Smith 
> wrote:
> > 
> >  -1 to (1) and (2).
> > 
> >  I think merge commits are appropriate in certain circumstances,
> > so I
> > > don't
> >  think we should make a blanket restriction. In fact I think our
> > >>> release
> >  process involves some merges.
> > 
> >  I think setting standards on what is reasonable to be an
> > individual
> > > commit
> >  will do a lot more to clean up our history than

Re: [DISCUSS] Proposal to require linear commit history on develop

[Blake Bender]

Is this a policy the native client team must abide by, as well?  It varies
slightly with what we've been doing, and all other things being equal I see
no reason for us to change that.  If I am to have any measure of control
over the nc repository, I will definitely enforce a 1:1 correspondence
between commits to develop and JIRA tickets.  IMO, if your refactoring in a
PR is sufficiently large or complex that it's difficult to tease it out
from the bug you're fixing or feature you're implementing, it merits its
own JIRA ticket and a separate PR.  If your "actual" fix then becomes
dependent on the refactored code, that's a price I'm willing to pay to keep
history clean.

On the other hand, I see no real value in squashing to a single commit
prior to submitting a PR, since your view of the changes on GitHub is
essentially the same either way.  We haven't enforced this on the nc repo,
and I'd prefer to keep it that way.

Thanks,

Blake


On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao  wrote:

> Merge commit is the new contributor's default setting. When we are merging
> new contributor's PR, since we are so used to THINKING "squash-and-merge"
> is the default, we forgot to check what the button really says when we are
> merging other people's PR.
>
> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt 
> wrote:
>
> > I'm a proponent of using squash-and-merge, and once a person has chosen
> > this option once it comes up by default afterwards...
> >
> > Owen,  I don't think you have consensus to put forth this PR, there are
> -1s
> > above... (early voting)
> >
> > maybe we'll be better off socializing the norm of squash-and-merge and
> > gaining a natural consensus that way...
> >
> > On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols 
> wrote:
> >
> > > The proposed action manifests as a commit to the Geode git repository,
> so
> > > a PR is an acceptable vehicle for voting in this case.
> > >
> > > > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
> bschucha...@pivotal.io>
> > > wrote:
> > > >
> > > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS
> thread
> > > and a request to "vote" using a PR.  This all seems out of order to me.
> > > Our votes are supposed to be on the email list, aren't they? and I
> > haven't
> > > seen a VOTE request.
> > > >
> > > > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> > > >> +1
> > > >>
> > > >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
> > > wrote:
> > > >>
> > > >>> Based on the feedback so far, I will amend the proposal to drop
> item
> > > 2).
> > > >>> Therefore, the current ability to create merge commits using
> > > command-line
> > > >>> git will remain available.
> > > >>>
> > > >>> The proposal as amended is now:
> > >  Change GitHub settings to make "Squash and merge" the default
> > >  (by removing “Create a merge commit” option).
> > > 
> > >  Update the PR template to change the text "Is your initial
> > > contribution
> > >  a single, squashed commit” to “Is your initial contribution
> squashed
> > > for
> > >  ease of review (e.g. a single commit, or a ‘refactoring’ commit
> > plus a
> > >  ‘fix’ commit)"
> > > >>>
> > > >>> As Naba suggested, we can try it, and if we don’t like it, it’s
> > simple
> > > to
> > > >>> revert.
> > > >>>
> > > >>> I’ve create a PR for the proposed change here:
> > > >>> https://github.com/apache/geode/pull/4513
> > > >>>
> > > >>> Please use the PR to vote for against this proposal.  It will not
> be
> > > >>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that
> time)
> > > >>>
> > >  On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> > > 
> > >  +1
> > > 
> > >  On Fri 20 Dec 2019 at 16:18, Owen Nichols 
> > > wrote:
> > > 
> > > > Hi Bruce, this proposal will not waste a single second of your
> > > time.  It
> > > > just prevents people from accidentally pressing a button that we
> > have
> > > > already agreed should never be pressed, but because we never
> > > configured
> > > >>> our
> > > > GitHub to match our stated policy, is currently the default.
> > > >
> > > > However, it will save a lot of time and frustration for anyone
> > > needing
> > > >>> to
> > > > bisect failures, revert, or cherry-pick changes, which has merit
> > > even if
> > > > you personally never do any of those three things.
> > > >
> > > > Please start a separate thread if you would like to revisit the
> > > >>> community
> > > > decision to require passing PR checks.
> > > >
> > > >> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt <
> > > bschucha...@pivotal.io>
> > > > wrote:
> > > >> I agree with Jake.  I would go further by saying that I see very
> > > little
> > > > merit in this proposal.  I think we're getting more and more
> > > >>> bureaucratic
> > > > in our process and that it stifles productivity.  I was recently
> > > forced
> > > >>> to
> > > > spend three days fixing tests in which I had changed an

Re: [DISCUSS] Proposal to require linear commit history on develop

[Aaron Lindsey]

Just to be clear, this proposal wouldn't require anyone to squash their commits 
before merging a PR. All it’s saying is that if you do have multiple commits in 
your PR, you would have to rebase those commits onto develop before merging to 
ensure a linear commit history.

- Aaron

> On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
> 
>  

Re: [DISCUSS] Proposal to require linear commit history on develop

[Anthony Baker]

Whether we are talking about the geode/ repository or the geode-native/ 
repository we are all one GEODE community.

The idea of a native client team may matter in some contexts but in this space 
we are all GEODE contributors.

Adopting a common approach across our repos will make it easier for new 
contributors to engage, learn our norms, and join our efforts.

$0.02,
Anthony


> On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
> 
> Is this a policy the native client team must abide by, as well?  It varies
> slightly with what we've been doing, and all other things being equal I see
> no reason for us to change that.  If I am to have any measure of control
> over the nc repository, I will definitely enforce a 1:1 correspondence
> between commits to develop and JIRA tickets.  IMO, if your refactoring in a
> PR is sufficiently large or complex that it's difficult to tease it out
> from the bug you're fixing or feature you're implementing, it merits its
> own JIRA ticket and a separate PR.  If your "actual" fix then becomes
> dependent on the refactored code, that's a price I'm willing to pay to keep
> history clean.
> 
> On the other hand, I see no real value in squashing to a single commit
> prior to submitting a PR, since your view of the changes on GitHub is
> essentially the same either way.  We haven't enforced this on the nc repo,
> and I'd prefer to keep it that way.
> 
> Thanks,
> 
> Blake
> 
> 
> On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao  wrote:
> 
>> Merge commit is the new contributor's default setting. When we are merging
>> new contributor's PR, since we are so used to THINKING "squash-and-merge"
>> is the default, we forgot to check what the button really says when we are
>> merging other people's PR.
>> 
>> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt 
>> wrote:
>> 
>>> I'm a proponent of using squash-and-merge, and once a person has chosen
>>> this option once it comes up by default afterwards...
>>> 
>>> Owen,  I don't think you have consensus to put forth this PR, there are
>> -1s
>>> above... (early voting)
>>> 
>>> maybe we'll be better off socializing the norm of squash-and-merge and
>>> gaining a natural consensus that way...
>>> 
>>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols 
>> wrote:
>>> 
 The proposed action manifests as a commit to the Geode git repository,
>> so
 a PR is an acceptable vehicle for voting in this case.
 
> On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
>> bschucha...@pivotal.io>
 wrote:
> 
> I see a lot of plus-ones and a "voting deadline" on this DISCUSS
>> thread
 and a request to "vote" using a PR.  This all seems out of order to me.
 Our votes are supposed to be on the email list, aren't they? and I
>>> haven't
 seen a VOTE request.
> 
> On 12/20/19 9:33 AM, Nabarun Nag wrote:
>> +1
>> 
>> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
 wrote:
>> 
>>> Based on the feedback so far, I will amend the proposal to drop
>> item
 2).
>>> Therefore, the current ability to create merge commits using
 command-line
>>> git will remain available.
>>> 
>>> The proposal as amended is now:
 Change GitHub settings to make "Squash and merge" the default
 (by removing “Create a merge commit” option).
 
 Update the PR template to change the text "Is your initial
 contribution
 a single, squashed commit” to “Is your initial contribution
>> squashed
 for
 ease of review (e.g. a single commit, or a ‘refactoring’ commit
>>> plus a
 ‘fix’ commit)"
>>> 
>>> As Naba suggested, we can try it, and if we don’t like it, it’s
>>> simple
 to
>>> revert.
>>> 
>>> I’ve create a PR for the proposed change here:
>>> https://github.com/apache/geode/pull/4513
>>> 
>>> Please use the PR to vote for against this proposal.  It will not
>> be
>>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that
>> time)
>>> 
 On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
 
 +1
 
 On Fri 20 Dec 2019 at 16:18, Owen Nichols 
 wrote:
 
> Hi Bruce, this proposal will not waste a single second of your
 time.  It
> just prevents people from accidentally pressing a button that we
>>> have
> already agreed should never be pressed, but because we never
 configured
>>> our
> GitHub to match our stated policy, is currently the default.
> 
> However, it will save a lot of time and frustration for anyone
 needing
>>> to
> bisect failures, revert, or cherry-pick changes, which has merit
 even if
> you personally never do any of those three things.
> 
> Please start a separate thread if you would like to revisit the
>>> community
> decision to require passing PR checks.
> 
>> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt <
 

Re: [DISCUSS] Proposal to require linear commit history on develop

[Blake Bender]

Very well, then, I'll abide by the group consensus but am on the record as:
* strongly opposed to multi-commit PRs, because I believe it clutters
history, and
* also not a big fan of forcing devs to rebase and squash prior to
submitting a PR.  IMO this is busy work, and *may* in some small minority
of cases hide information that would be useful to reviewers.

Thanks,

Blake


On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker  wrote:

> Whether we are talking about the geode/ repository or the geode-native/
> repository we are all one GEODE community.
>
> The idea of a native client team may matter in some contexts but in this
> space we are all GEODE contributors.
>
> Adopting a common approach across our repos will make it easier for new
> contributors to engage, learn our norms, and join our efforts.
>
> $0.02,
> Anthony
>
>
> > On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
> >
> > Is this a policy the native client team must abide by, as well?  It
> varies
> > slightly with what we've been doing, and all other things being equal I
> see
> > no reason for us to change that.  If I am to have any measure of control
> > over the nc repository, I will definitely enforce a 1:1 correspondence
> > between commits to develop and JIRA tickets.  IMO, if your refactoring
> in a
> > PR is sufficiently large or complex that it's difficult to tease it out
> > from the bug you're fixing or feature you're implementing, it merits its
> > own JIRA ticket and a separate PR.  If your "actual" fix then becomes
> > dependent on the refactored code, that's a price I'm willing to pay to
> keep
> > history clean.
> >
> > On the other hand, I see no real value in squashing to a single commit
> > prior to submitting a PR, since your view of the changes on GitHub is
> > essentially the same either way.  We haven't enforced this on the nc
> repo,
> > and I'd prefer to keep it that way.
> >
> > Thanks,
> >
> > Blake
> >
> >
> > On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao  wrote:
> >
> >> Merge commit is the new contributor's default setting. When we are
> merging
> >> new contributor's PR, since we are so used to THINKING
> "squash-and-merge"
> >> is the default, we forgot to check what the button really says when we
> are
> >> merging other people's PR.
> >>
> >> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt <
> eburgha...@pivotal.io>
> >> wrote:
> >>
> >>> I'm a proponent of using squash-and-merge, and once a person has chosen
> >>> this option once it comes up by default afterwards...
> >>>
> >>> Owen,  I don't think you have consensus to put forth this PR, there are
> >> -1s
> >>> above... (early voting)
> >>>
> >>> maybe we'll be better off socializing the norm of squash-and-merge and
> >>> gaining a natural consensus that way...
> >>>
> >>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols 
> >> wrote:
> >>>
>  The proposed action manifests as a commit to the Geode git repository,
> >> so
>  a PR is an acceptable vehicle for voting in this case.
> 
> > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
> >> bschucha...@pivotal.io>
>  wrote:
> >
> > I see a lot of plus-ones and a "voting deadline" on this DISCUSS
> >> thread
>  and a request to "vote" using a PR.  This all seems out of order to
> me.
>  Our votes are supposed to be on the email list, aren't they? and I
> >>> haven't
>  seen a VOTE request.
> >
> > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> >> +1
> >>
> >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
>  wrote:
> >>
> >>> Based on the feedback so far, I will amend the proposal to drop
> >> item
>  2).
> >>> Therefore, the current ability to create merge commits using
>  command-line
> >>> git will remain available.
> >>>
> >>> The proposal as amended is now:
>  Change GitHub settings to make "Squash and merge" the default
>  (by removing “Create a merge commit” option).
> 
>  Update the PR template to change the text "Is your initial
>  contribution
>  a single, squashed commit” to “Is your initial contribution
> >> squashed
>  for
>  ease of review (e.g. a single commit, or a ‘refactoring’ commit
> >>> plus a
>  ‘fix’ commit)"
> >>>
> >>> As Naba suggested, we can try it, and if we don’t like it, it’s
> >>> simple
>  to
> >>> revert.
> >>>
> >>> I’ve create a PR for the proposed change here:
> >>> https://github.com/apache/geode/pull/4513
> >>>
> >>> Please use the PR to vote for against this proposal.  It will not
> >> be
> >>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that
> >> time)
> >>>
>  On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> 
>  +1
> 
>  On Fri 20 Dec 2019 at 16:18, Owen Nichols 
>  wrote:
> 
> > Hi Bruce, this proposal will not waste a single second of your
>  time.  It
> > just prevents people from accidentally

Re: [DISCUSS] Proposal to require linear commit history on develop

[Nabarun Nag]

Just to reiterate. Nothing changes in the workflow of a developer. It's
just in the end, when all the reviews are done and all the tests are
passing, then the button to click in the Github web UI is "Squash and
Merge". That's all.

Regards
Naba



On Fri, Dec 20, 2019 at 11:55 AM Blake Bender  wrote:

> Very well, then, I'll abide by the group consensus but am on the record as:
> * strongly opposed to multi-commit PRs, because I believe it clutters
> history, and
> * also not a big fan of forcing devs to rebase and squash prior to
> submitting a PR.  IMO this is busy work, and *may* in some small minority
> of cases hide information that would be useful to reviewers.
>
> Thanks,
>
> Blake
>
>
> On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker  wrote:
>
> > Whether we are talking about the geode/ repository or the geode-native/
> > repository we are all one GEODE community.
> >
> > The idea of a native client team may matter in some contexts but in this
> > space we are all GEODE contributors.
> >
> > Adopting a common approach across our repos will make it easier for new
> > contributors to engage, learn our norms, and join our efforts.
> >
> > $0.02,
> > Anthony
> >
> >
> > > On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
> > >
> > > Is this a policy the native client team must abide by, as well?  It
> > varies
> > > slightly with what we've been doing, and all other things being equal I
> > see
> > > no reason for us to change that.  If I am to have any measure of
> control
> > > over the nc repository, I will definitely enforce a 1:1 correspondence
> > > between commits to develop and JIRA tickets.  IMO, if your refactoring
> > in a
> > > PR is sufficiently large or complex that it's difficult to tease it out
> > > from the bug you're fixing or feature you're implementing, it merits
> its
> > > own JIRA ticket and a separate PR.  If your "actual" fix then becomes
> > > dependent on the refactored code, that's a price I'm willing to pay to
> > keep
> > > history clean.
> > >
> > > On the other hand, I see no real value in squashing to a single commit
> > > prior to submitting a PR, since your view of the changes on GitHub is
> > > essentially the same either way.  We haven't enforced this on the nc
> > repo,
> > > and I'd prefer to keep it that way.
> > >
> > > Thanks,
> > >
> > > Blake
> > >
> > >
> > > On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao 
> wrote:
> > >
> > >> Merge commit is the new contributor's default setting. When we are
> > merging
> > >> new contributor's PR, since we are so used to THINKING
> > "squash-and-merge"
> > >> is the default, we forgot to check what the button really says when we
> > are
> > >> merging other people's PR.
> > >>
> > >> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt <
> > eburgha...@pivotal.io>
> > >> wrote:
> > >>
> > >>> I'm a proponent of using squash-and-merge, and once a person has
> chosen
> > >>> this option once it comes up by default afterwards...
> > >>>
> > >>> Owen,  I don't think you have consensus to put forth this PR, there
> are
> > >> -1s
> > >>> above... (early voting)
> > >>>
> > >>> maybe we'll be better off socializing the norm of squash-and-merge
> and
> > >>> gaining a natural consensus that way...
> > >>>
> > >>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols 
> > >> wrote:
> > >>>
> >  The proposed action manifests as a commit to the Geode git
> repository,
> > >> so
> >  a PR is an acceptable vehicle for voting in this case.
> > 
> > > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
> > >> bschucha...@pivotal.io>
> >  wrote:
> > >
> > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS
> > >> thread
> >  and a request to "vote" using a PR.  This all seems out of order to
> > me.
> >  Our votes are supposed to be on the email list, aren't they? and I
> > >>> haven't
> >  seen a VOTE request.
> > >
> > > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> > >> +1
> > >>
> > >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols  >
> >  wrote:
> > >>
> > >>> Based on the feedback so far, I will amend the proposal to drop
> > >> item
> >  2).
> > >>> Therefore, the current ability to create merge commits using
> >  command-line
> > >>> git will remain available.
> > >>>
> > >>> The proposal as amended is now:
> >  Change GitHub settings to make "Squash and merge" the default
> >  (by removing “Create a merge commit” option).
> > 
> >  Update the PR template to change the text "Is your initial
> >  contribution
> >  a single, squashed commit” to “Is your initial contribution
> > >> squashed
> >  for
> >  ease of review (e.g. a single commit, or a ‘refactoring’ commit
> > >>> plus a
> >  ‘fix’ commit)"
> > >>>
> > >>> As Naba suggested, we can try it, and if we don’t like it, it’s
> > >>> simple
> >  to
> > >>> revert.
> > >>>
> > >>> I’ve create a PR for the

Re: [DISCUSS] Proposal to require linear commit history on develop

[Owen Nichols]

We are not trying to force devs to squash prior to committing a PR.  There was 
old language in the PR template that stated this, which we are relaxing to 
emphasize submitting your PR in a  way that is most amenable to review.  The 
geode-native repo does not appear to have a .github/PULL_REQUEST_TEMPLATE.md 
,
 but you may wish to consider adopting Geode’s and/or proposing changes that 
could be applied to both repos.

Squash and merge remains the most common and most acceptable way to merge PRs, 
but the Geode community felt strongly that some developers might have a good 
reason to group multiple commits in one PR.  It’s entirely possible each commit 
could have it’s own JIRA, preserving your 1:1 expectation after rebase&merge to 
develop.  The geode-native repo does not appear to have a .a 
sf.yaml,
 but you may wish to consider adopting the one currently proposed for Geode 
and/or proposing changes that could be applied to both repos.


> On Dec 20, 2019, at 11:54 AM, Blake Bender  wrote:
> 
> Very well, then, I'll abide by the group consensus but am on the record as:
> * strongly opposed to multi-commit PRs, because I believe it clutters
> history, and
> * also not a big fan of forcing devs to rebase and squash prior to
> submitting a PR.  IMO this is busy work, and *may* in some small minority
> of cases hide information that would be useful to reviewers.
> 
> Thanks,
> 
> Blake
> 
> 
> On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker  wrote:
> 
>> Whether we are talking about the geode/ repository or the geode-native/
>> repository we are all one GEODE community.
>> 
>> The idea of a native client team may matter in some contexts but in this
>> space we are all GEODE contributors.
>> 
>> Adopting a common approach across our repos will make it easier for new
>> contributors to engage, learn our norms, and join our efforts.
>> 
>> $0.02,
>> Anthony
>> 
>> 
>>> On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
>>> 
>>> Is this a policy the native client team must abide by, as well?  It
>> varies
>>> slightly with what we've been doing, and all other things being equal I
>> see
>>> no reason for us to change that.  If I am to have any measure of control
>>> over the nc repository, I will definitely enforce a 1:1 correspondence
>>> between commits to develop and JIRA tickets.  IMO, if your refactoring
>> in a
>>> PR is sufficiently large or complex that it's difficult to tease it out
>>> from the bug you're fixing or feature you're implementing, it merits its
>>> own JIRA ticket and a separate PR.  If your "actual" fix then becomes
>>> dependent on the refactored code, that's a price I'm willing to pay to
>> keep
>>> history clean.
>>> 
>>> On the other hand, I see no real value in squashing to a single commit
>>> prior to submitting a PR, since your view of the changes on GitHub is
>>> essentially the same either way.  We haven't enforced this on the nc
>> repo,
>>> and I'd prefer to keep it that way.
>>> 
>>> Thanks,
>>> 
>>> Blake
>>> 
>>> 
>>> On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao  wrote:
>>> 
 Merge commit is the new contributor's default setting. When we are
>> merging
 new contributor's PR, since we are so used to THINKING
>> "squash-and-merge"
 is the default, we forgot to check what the button really says when we
>> are
 merging other people's PR.
 
 On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt <
>> eburgha...@pivotal.io>
 wrote:
 
> I'm a proponent of using squash-and-merge, and once a person has chosen
> this option once it comes up by default afterwards...
> 
> Owen,  I don't think you have consensus to put forth this PR, there are
 -1s
> above... (early voting)
> 
> maybe we'll be better off socializing the norm of squash-and-merge and
> gaining a natural consensus that way...
> 
> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols 
 wrote:
> 
>> The proposed action manifests as a commit to the Geode git repository,
 so
>> a PR is an acceptable vehicle for voting in this case.
>> 
>>> On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
 bschucha...@pivotal.io>
>> wrote:
>>> 
>>> I see a lot of plus-ones and a "voting deadline" on this DISCUSS
 thread
>> and a request to "vote" using a PR.  This all seems out of order to
>> me.
>> Our votes are supposed to be on the email list, aren't they? and I
> haven't
>> seen a VOTE request.
>>> 
>>> On 12/20/19 9:33 AM, Nabarun Nag wrote:
 +1
 
 On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
>> wrote:
 
> Based on the feedback so far, I will amend the proposal to drop
 item
>> 2).
> Therefore, the current ability to create merge commits using
>> command-line
> git will remai

Re: [DISCUSS] Proposal to require linear commit history on develop

[Dan Smith]

I'll change to -0.

I think merge commits are a nice way to record history in some cases, and
can also be a way to avoid messy conflicts that come with rebase. Merge
commits also preserve authorship of commits (compared to squash-merge),
which I think is valuable as an open source community that is trying to
keep track of outside contributions.

That said, if the rest of y'all feel it will help to disable the button, I
won't stand in the way.

-Dan

On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker  wrote:

> Whether we are talking about the geode/ repository or the geode-native/
> repository we are all one GEODE community.
>
> The idea of a native client team may matter in some contexts but in this
> space we are all GEODE contributors.
>
> Adopting a common approach across our repos will make it easier for new
> contributors to engage, learn our norms, and join our efforts.
>
> $0.02,
> Anthony
>
>
> > On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
> >
> > Is this a policy the native client team must abide by, as well?  It
> varies
> > slightly with what we've been doing, and all other things being equal I
> see
> > no reason for us to change that.  If I am to have any measure of control
> > over the nc repository, I will definitely enforce a 1:1 correspondence
> > between commits to develop and JIRA tickets.  IMO, if your refactoring
> in a
> > PR is sufficiently large or complex that it's difficult to tease it out
> > from the bug you're fixing or feature you're implementing, it merits its
> > own JIRA ticket and a separate PR.  If your "actual" fix then becomes
> > dependent on the refactored code, that's a price I'm willing to pay to
> keep
> > history clean.
> >
> > On the other hand, I see no real value in squashing to a single commit
> > prior to submitting a PR, since your view of the changes on GitHub is
> > essentially the same either way.  We haven't enforced this on the nc
> repo,
> > and I'd prefer to keep it that way.
> >
> > Thanks,
> >
> > Blake
> >
> >
> > On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao  wrote:
> >
> >> Merge commit is the new contributor's default setting. When we are
> merging
> >> new contributor's PR, since we are so used to THINKING
> "squash-and-merge"
> >> is the default, we forgot to check what the button really says when we
> are
> >> merging other people's PR.
> >>
> >> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt <
> eburgha...@pivotal.io>
> >> wrote:
> >>
> >>> I'm a proponent of using squash-and-merge, and once a person has chosen
> >>> this option once it comes up by default afterwards...
> >>>
> >>> Owen,  I don't think you have consensus to put forth this PR, there are
> >> -1s
> >>> above... (early voting)
> >>>
> >>> maybe we'll be better off socializing the norm of squash-and-merge and
> >>> gaining a natural consensus that way...
> >>>
> >>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols 
> >> wrote:
> >>>
>  The proposed action manifests as a commit to the Geode git repository,
> >> so
>  a PR is an acceptable vehicle for voting in this case.
> 
> > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
> >> bschucha...@pivotal.io>
>  wrote:
> >
> > I see a lot of plus-ones and a "voting deadline" on this DISCUSS
> >> thread
>  and a request to "vote" using a PR.  This all seems out of order to
> me.
>  Our votes are supposed to be on the email list, aren't they? and I
> >>> haven't
>  seen a VOTE request.
> >
> > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> >> +1
> >>
> >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
>  wrote:
> >>
> >>> Based on the feedback so far, I will amend the proposal to drop
> >> item
>  2).
> >>> Therefore, the current ability to create merge commits using
>  command-line
> >>> git will remain available.
> >>>
> >>> The proposal as amended is now:
>  Change GitHub settings to make "Squash and merge" the default
>  (by removing “Create a merge commit” option).
> 
>  Update the PR template to change the text "Is your initial
>  contribution
>  a single, squashed commit” to “Is your initial contribution
> >> squashed
>  for
>  ease of review (e.g. a single commit, or a ‘refactoring’ commit
> >>> plus a
>  ‘fix’ commit)"
> >>>
> >>> As Naba suggested, we can try it, and if we don’t like it, it’s
> >>> simple
>  to
> >>> revert.
> >>>
> >>> I’ve create a PR for the proposed change here:
> >>> https://github.com/apache/geode/pull/4513
> >>>
> >>> Please use the PR to vote for against this proposal.  It will not
> >> be
> >>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that
> >> time)
> >>>
>  On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> 
>  +1
> 
>  On Fri 20 Dec 2019 at 16:18, Owen Nichols 
>  wrote:
> 
> > Hi Bruce, this proposal will not waste a single second 

Re: [DISCUSS] Proposal to require linear commit history on develop

[Blake Bender]

This was the part I was referring to, from the existing PR template:

> Update the PR template to change the text "Is your initial contribution
> a single, squashed commit” to “Is your initial contribution squashed for
> ease of review (e.g. a single commit, or a ‘refactoring’ commit plus a
> ‘fix’ commit)"

Either of these, while I guess not strictly required(?), is a change from
what SOP has been for native client.  We rebase onto the current develop
when submitting a PR, but don't pre-squash commits in any meaningful way,
because it's unnecessary.

The rest is fine - changing the default option to Squash & Merge is the
right thing to do IMO.

Thanks,

Blake


On Fri, Dec 20, 2019 at 12:00 PM Nabarun Nag  wrote:

> Just to reiterate. Nothing changes in the workflow of a developer. It's
> just in the end, when all the reviews are done and all the tests are
> passing, then the button to click in the Github web UI is "Squash and
> Merge". That's all.
>
> Regards
> Naba
>
>
>
> On Fri, Dec 20, 2019 at 11:55 AM Blake Bender  wrote:
>
> > Very well, then, I'll abide by the group consensus but am on the record
> as:
> > * strongly opposed to multi-commit PRs, because I believe it clutters
> > history, and
> > * also not a big fan of forcing devs to rebase and squash prior to
> > submitting a PR.  IMO this is busy work, and *may* in some small minority
> > of cases hide information that would be useful to reviewers.
> >
> > Thanks,
> >
> > Blake
> >
> >
> > On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker 
> wrote:
> >
> > > Whether we are talking about the geode/ repository or the geode-native/
> > > repository we are all one GEODE community.
> > >
> > > The idea of a native client team may matter in some contexts but in
> this
> > > space we are all GEODE contributors.
> > >
> > > Adopting a common approach across our repos will make it easier for new
> > > contributors to engage, learn our norms, and join our efforts.
> > >
> > > $0.02,
> > > Anthony
> > >
> > >
> > > > On Dec 20, 2019, at 11:32 AM, Blake Bender 
> wrote:
> > > >
> > > > Is this a policy the native client team must abide by, as well?  It
> > > varies
> > > > slightly with what we've been doing, and all other things being
> equal I
> > > see
> > > > no reason for us to change that.  If I am to have any measure of
> > control
> > > > over the nc repository, I will definitely enforce a 1:1
> correspondence
> > > > between commits to develop and JIRA tickets.  IMO, if your
> refactoring
> > > in a
> > > > PR is sufficiently large or complex that it's difficult to tease it
> out
> > > > from the bug you're fixing or feature you're implementing, it merits
> > its
> > > > own JIRA ticket and a separate PR.  If your "actual" fix then becomes
> > > > dependent on the refactored code, that's a price I'm willing to pay
> to
> > > keep
> > > > history clean.
> > > >
> > > > On the other hand, I see no real value in squashing to a single
> commit
> > > > prior to submitting a PR, since your view of the changes on GitHub is
> > > > essentially the same either way.  We haven't enforced this on the nc
> > > repo,
> > > > and I'd prefer to keep it that way.
> > > >
> > > > Thanks,
> > > >
> > > > Blake
> > > >
> > > >
> > > > On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao 
> > wrote:
> > > >
> > > >> Merge commit is the new contributor's default setting. When we are
> > > merging
> > > >> new contributor's PR, since we are so used to THINKING
> > > "squash-and-merge"
> > > >> is the default, we forgot to check what the button really says when
> we
> > > are
> > > >> merging other people's PR.
> > > >>
> > > >> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt <
> > > eburgha...@pivotal.io>
> > > >> wrote:
> > > >>
> > > >>> I'm a proponent of using squash-and-merge, and once a person has
> > chosen
> > > >>> this option once it comes up by default afterwards...
> > > >>>
> > > >>> Owen,  I don't think you have consensus to put forth this PR, there
> > are
> > > >> -1s
> > > >>> above... (early voting)
> > > >>>
> > > >>> maybe we'll be better off socializing the norm of squash-and-merge
> > and
> > > >>> gaining a natural consensus that way...
> > > >>>
> > > >>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols  >
> > > >> wrote:
> > > >>>
> > >  The proposed action manifests as a commit to the Geode git
> > repository,
> > > >> so
> > >  a PR is an acceptable vehicle for voting in this case.
> > > 
> > > > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
> > > >> bschucha...@pivotal.io>
> > >  wrote:
> > > >
> > > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS
> > > >> thread
> > >  and a request to "vote" using a PR.  This all seems out of order
> to
> > > me.
> > >  Our votes are supposed to be on the email list, aren't they? and I
> > > >>> haven't
> > >  seen a VOTE request.
> > > >
> > > > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> > > >> +1
> > > >>
> > > >> On Fri, Dec 20, 2019 at 

Re: [DISCUSS] Proposal to require linear commit history on develop

[Nabarun Nag]

Hi Dan,

When we do squash merge all the commit messages are preserved and also the
co-authored tag works when we do squash merge.
So the authorship and history are preserved.

In my own personal experience and reverts and pinpointing regression
failures are tough when the commits are spread out. Also, reverts are
easier when it is just one commit while we are bisecting failures.


Regards
Naba

On Fri, Dec 20, 2019 at 12:07 PM Dan Smith  wrote:

> I'll change to -0.
>
> I think merge commits are a nice way to record history in some cases, and
> can also be a way to avoid messy conflicts that come with rebase. Merge
> commits also preserve authorship of commits (compared to squash-merge),
> which I think is valuable as an open source community that is trying to
> keep track of outside contributions.
>
> That said, if the rest of y'all feel it will help to disable the button, I
> won't stand in the way.
>
> -Dan
>
> On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker  wrote:
>
> > Whether we are talking about the geode/ repository or the geode-native/
> > repository we are all one GEODE community.
> >
> > The idea of a native client team may matter in some contexts but in this
> > space we are all GEODE contributors.
> >
> > Adopting a common approach across our repos will make it easier for new
> > contributors to engage, learn our norms, and join our efforts.
> >
> > $0.02,
> > Anthony
> >
> >
> > > On Dec 20, 2019, at 11:32 AM, Blake Bender  wrote:
> > >
> > > Is this a policy the native client team must abide by, as well?  It
> > varies
> > > slightly with what we've been doing, and all other things being equal I
> > see
> > > no reason for us to change that.  If I am to have any measure of
> control
> > > over the nc repository, I will definitely enforce a 1:1 correspondence
> > > between commits to develop and JIRA tickets.  IMO, if your refactoring
> > in a
> > > PR is sufficiently large or complex that it's difficult to tease it out
> > > from the bug you're fixing or feature you're implementing, it merits
> its
> > > own JIRA ticket and a separate PR.  If your "actual" fix then becomes
> > > dependent on the refactored code, that's a price I'm willing to pay to
> > keep
> > > history clean.
> > >
> > > On the other hand, I see no real value in squashing to a single commit
> > > prior to submitting a PR, since your view of the changes on GitHub is
> > > essentially the same either way.  We haven't enforced this on the nc
> > repo,
> > > and I'd prefer to keep it that way.
> > >
> > > Thanks,
> > >
> > > Blake
> > >
> > >
> > > On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao 
> wrote:
> > >
> > >> Merge commit is the new contributor's default setting. When we are
> > merging
> > >> new contributor's PR, since we are so used to THINKING
> > "squash-and-merge"
> > >> is the default, we forgot to check what the button really says when we
> > are
> > >> merging other people's PR.
> > >>
> > >> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt <
> > eburgha...@pivotal.io>
> > >> wrote:
> > >>
> > >>> I'm a proponent of using squash-and-merge, and once a person has
> chosen
> > >>> this option once it comes up by default afterwards...
> > >>>
> > >>> Owen,  I don't think you have consensus to put forth this PR, there
> are
> > >> -1s
> > >>> above... (early voting)
> > >>>
> > >>> maybe we'll be better off socializing the norm of squash-and-merge
> and
> > >>> gaining a natural consensus that way...
> > >>>
> > >>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols 
> > >> wrote:
> > >>>
> >  The proposed action manifests as a commit to the Geode git
> repository,
> > >> so
> >  a PR is an acceptable vehicle for voting in this case.
> > 
> > > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
> > >> bschucha...@pivotal.io>
> >  wrote:
> > >
> > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS
> > >> thread
> >  and a request to "vote" using a PR.  This all seems out of order to
> > me.
> >  Our votes are supposed to be on the email list, aren't they? and I
> > >>> haven't
> >  seen a VOTE request.
> > >
> > > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> > >> +1
> > >>
> > >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols  >
> >  wrote:
> > >>
> > >>> Based on the feedback so far, I will amend the proposal to drop
> > >> item
> >  2).
> > >>> Therefore, the current ability to create merge commits using
> >  command-line
> > >>> git will remain available.
> > >>>
> > >>> The proposal as amended is now:
> >  Change GitHub settings to make "Squash and merge" the default
> >  (by removing “Create a merge commit” option).
> > 
> >  Update the PR template to change the text "Is your initial
> >  contribution
> >  a single, squashed commit” to “Is your initial contribution
> > >> squashed
> >  for
> >  ease of review (e.g. a single commit, or a ‘refactoring’ commit
>

Re: [DISCUSS] Proposal to require linear commit history on develop

[Blake Bender]

Just FWIW, the situation described of having multiple commits in a single
PR with separate associated JIRA tickets is still kind of problematic.  It
could well be the case that the commits are interdependent, thus when
bisecting etc it's still not possible to revert the fix for a single
bug/feature/whatever atomically.  It's all good, though, I'm satisfied no
one's forcing me to adopt practice I'm opposed to.  Apologies for getting
my feathers a little ruffled, or if I ruffled anyone else's in return.

Thanks,

Blake


On Fri, Dec 20, 2019 at 12:18 PM Nabarun Nag  wrote:

> Hi Dan,
>
> When we do squash merge all the commit messages are preserved and also the
> co-authored tag works when we do squash merge.
> So the authorship and history are preserved.
>
> In my own personal experience and reverts and pinpointing regression
> failures are tough when the commits are spread out. Also, reverts are
> easier when it is just one commit while we are bisecting failures.
>
>
> Regards
> Naba
>
> On Fri, Dec 20, 2019 at 12:07 PM Dan Smith  wrote:
>
> > I'll change to -0.
> >
> > I think merge commits are a nice way to record history in some cases, and
> > can also be a way to avoid messy conflicts that come with rebase. Merge
> > commits also preserve authorship of commits (compared to squash-merge),
> > which I think is valuable as an open source community that is trying to
> > keep track of outside contributions.
> >
> > That said, if the rest of y'all feel it will help to disable the button,
> I
> > won't stand in the way.
> >
> > -Dan
> >
> > On Fri, Dec 20, 2019 at 11:50 AM Anthony Baker 
> wrote:
> >
> > > Whether we are talking about the geode/ repository or the geode-native/
> > > repository we are all one GEODE community.
> > >
> > > The idea of a native client team may matter in some contexts but in
> this
> > > space we are all GEODE contributors.
> > >
> > > Adopting a common approach across our repos will make it easier for new
> > > contributors to engage, learn our norms, and join our efforts.
> > >
> > > $0.02,
> > > Anthony
> > >
> > >
> > > > On Dec 20, 2019, at 11:32 AM, Blake Bender 
> wrote:
> > > >
> > > > Is this a policy the native client team must abide by, as well?  It
> > > varies
> > > > slightly with what we've been doing, and all other things being
> equal I
> > > see
> > > > no reason for us to change that.  If I am to have any measure of
> > control
> > > > over the nc repository, I will definitely enforce a 1:1
> correspondence
> > > > between commits to develop and JIRA tickets.  IMO, if your
> refactoring
> > > in a
> > > > PR is sufficiently large or complex that it's difficult to tease it
> out
> > > > from the bug you're fixing or feature you're implementing, it merits
> > its
> > > > own JIRA ticket and a separate PR.  If your "actual" fix then becomes
> > > > dependent on the refactored code, that's a price I'm willing to pay
> to
> > > keep
> > > > history clean.
> > > >
> > > > On the other hand, I see no real value in squashing to a single
> commit
> > > > prior to submitting a PR, since your view of the changes on GitHub is
> > > > essentially the same either way.  We haven't enforced this on the nc
> > > repo,
> > > > and I'd prefer to keep it that way.
> > > >
> > > > Thanks,
> > > >
> > > > Blake
> > > >
> > > >
> > > > On Fri, Dec 20, 2019 at 10:29 AM Jinmei Liao 
> > wrote:
> > > >
> > > >> Merge commit is the new contributor's default setting. When we are
> > > merging
> > > >> new contributor's PR, since we are so used to THINKING
> > > "squash-and-merge"
> > > >> is the default, we forgot to check what the button really says when
> we
> > > are
> > > >> merging other people's PR.
> > > >>
> > > >> On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt <
> > > eburgha...@pivotal.io>
> > > >> wrote:
> > > >>
> > > >>> I'm a proponent of using squash-and-merge, and once a person has
> > chosen
> > > >>> this option once it comes up by default afterwards...
> > > >>>
> > > >>> Owen,  I don't think you have consensus to put forth this PR, there
> > are
> > > >> -1s
> > > >>> above... (early voting)
> > > >>>
> > > >>> maybe we'll be better off socializing the norm of squash-and-merge
> > and
> > > >>> gaining a natural consensus that way...
> > > >>>
> > > >>> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols  >
> > > >> wrote:
> > > >>>
> > >  The proposed action manifests as a commit to the Geode git
> > repository,
> > > >> so
> > >  a PR is an acceptable vehicle for voting in this case.
> > > 
> > > > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt <
> > > >> bschucha...@pivotal.io>
> > >  wrote:
> > > >
> > > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS
> > > >> thread
> > >  and a request to "vote" using a PR.  This all seems out of order
> to
> > > me.
> > >  Our votes are supposed to be on the email list, aren't they? and I
> > > >>> haven't
> > >  seen a VOTE request.
> > > >
> > > > On 12/20/19 9:33 A

Errored: apache/geode-examples#395 (rel/v1.11.0.RC4 - 9e6f13e)

[Travis CI]

Build Update for apache/geode-examples
-

Build: #395
Status: Errored

Duration: 1 min and 15 secs
Commit: 9e6f13e (rel/v1.11.0.RC4)
Author: Mark Hanson
Message: temporarily point to staging repo for CI purposes

View the changeset: 
https://github.com/apache/geode-examples/compare/rel/v1.11.0.RC4

View the full build log and details: 
https://travis-ci.org/apache/geode-examples/builds/627935089?utm_medium=notification&utm_source=email

--

You can unsubscribe from build emails from the apache/geode-examples repository 
going to 
https://travis-ci.org/account/preferences/unsubscribe?repository=11483039&utm_medium=notification&utm_source=email.
Or unsubscribe from *all* email updating your settings at 
https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification&utm_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.

[VOTE] Apache Geode 1.11.0.RC4

[Mark Hanson]

Subject: [VOTE] Apache Geode 1.11.0.RC4
Hello Geode Dev Community,

This is a release candidate for Apache Geode version 1.11.0.RC4.
Thanks to all the community members for their contributions to this release!

Please do a review and give your feedback, including the checks you performed.

Voting deadline:
3PM PST Wed, December 26 2019.

Please note that we are voting upon the source tag:
rel/v1.11.0.RC4

Release notes:
https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.11.0

Source and binary distributions:
https://dist.apache.org/repos/dist/dev/geode/1.11.0.RC4/

Maven staging repo:
https://repository.apache.org/content/repositories/orgapachegeode-1064

GitHub:
https://github.com/apache/geode/tree/rel/v1.11.0.RC4
https://github.com/apache/geode-examples/tree/rel/v1.11.0.RC4
https://github.com/apache/geode-native/tree/rel/v1.11.0.RC4

Pipelines:
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-11-0-main
https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-11-0-rc

Geode's KEYS file containing PGP keys we use to sign the release:
https://github.com/apache/geode/blob/develop/KEYS

Command to run geode-examples:
./gradlew 
-PgeodeReleaseUrl=https://dist.apache.org/repos/dist/dev/geode/1.11.0.RC4 
-PgeodeRepositoryUrl=https://repository.apache.org/content/repositories/orgapachegeode-1064
 build runAll

Regards
Mark Hanson

Re: [VOTE] Apache Geode 1.11.0.RC4

[Mark Hanson]

Given it is the holidays, perhaps more time is in order. I am bumping the 
voting deadline to Friday, December 27th, 2019.

Thanks,
Mark



> On Dec 20, 2019, at 2:46 PM, Mark Hanson  wrote:
> 
> Subject: [VOTE] Apache Geode 1.11.0.RC4
> Hello Geode Dev Community,
> 
> This is a release candidate for Apache Geode version 1.11.0.RC4.
> Thanks to all the community members for their contributions to this release!
> 
> Please do a review and give your feedback, including the checks you performed.
> 
> Voting deadline:
> 3PM PST Friday, December 27 2019.
> 
> Please note that we are voting upon the source tag:
> rel/v1.11.0.RC4
> 
> Release notes:
> https://cwiki.apache.org/confluence/display/GEODE/Release+Notes#ReleaseNotes-1.11.0
> 
> Source and binary distributions:
> https://dist.apache.org/repos/dist/dev/geode/1.11.0.RC4/
> 
> Maven staging repo:
> https://repository.apache.org/content/repositories/orgapachegeode-1064
> 
> GitHub:
> https://github.com/apache/geode/tree/rel/v1.11.0.RC4
> https://github.com/apache/geode-examples/tree/rel/v1.11.0.RC4
> https://github.com/apache/geode-native/tree/rel/v1.11.0.RC4
> 
> Pipelines:
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-11-0-main
> https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-release-1-11-0-rc
> 
> Geode's KEYS file containing PGP keys we use to sign the release:
> https://github.com/apache/geode/blob/develop/KEYS
> 
> Command to run geode-examples:
> ./gradlew 
> -PgeodeReleaseUrl=https://dist.apache.org/repos/dist/dev/geode/1.11.0.RC4 
> -PgeodeRepositoryUrl=https://repository.apache.org/content/repositories/orgapachegeode-1064
>  build runAll
> 
> Regards
> Mark Hanson
> 

Re: [DISCUSS] `status locator` command fail when locator's ssl is turned on

[Ernest Burghardt]

I like the approach Jens is suggesting, seems intuitive to me

On Thu, Dec 19, 2019 at 6:31 PM Jens Deppe  wrote:

> So it seems that the situation is something like this where I'm able to
> make the initial connection but then retrieving status fails:
>
> gfsh>connect --security-properties-file=../security.properties
>
> gfsh>status locator --name=locator1
> Server version response invalid: This could be the result of trying to
> connect a non-SSL-enabled client to an SSL-enabled locator.
>
>
> It would seem very weird if I have to provide additional connection params
> to the 'status' command if I've already provided them as part of the
> 'connect'. Could we not stash the connection properties (in the Gfsh
> instance object) for subsequent usage?
>
> --Jens
>
> On Wed, Dec 18, 2019 at 9:04 AM Jinmei Liao  wrote:
>
> > "status locator" command is broken on ssl enabled locators ever since we
> > fixed a bug that leaked the connection properties from one tcp socket
> > connection to another. Before that it would just magically work if we
> have
> > previously made a successfully tcp connection to that same locator, now
> we
> > are either required to find a way to specify the ssl properties when
> > running the `status locator` command or change what we want to report
> back
> > to the user.
> >
> > Here is what's happening now when we issue the command:
> >
> >1. it needs to retrieve two sets of info from locator: general info
> like
> >(pid, working dir, status, jvm args etc) and whether cluster
> > configuration
> >service is running or not.
> >2. when locator’s ssl is on, the retrieval of the cluster
> configuration
> >info will always fail since it’s using a tcp connection to get that
> info
> >and we currently don’t have the ssl security properties to connect.
> >3. when locator’s ssl is on, the retrieval of the general info will
> >mostly succeed except when user is only providing a host and port,
> > there we
> >would also need the ssl security properties in order to create an ssl
> >socket.
> >
> >
> > So we wither need to add an option for user to specify a
> > `--security-properties-file` option to include all the ssl connection
> > properties or change the behavior not to report cluster config status or
> > not allowing host/port combination. (I am not here long enough to
> > understand what users would use this command for, so this is just a
> random
> > suggestions).
> >
> > Comments/thoughts?
> >
> > --
> > Cheers
> >
> > Jinmei
> >
>

Passed: apache/geode-native#2275 (rel/v1.11.0.RC4 - 3199cae)

[Travis CI]

Build Update for apache/geode-native
-

Build: #2275
Status: Passed

Duration: 1 hr, 17 mins, and 3 secs
Commit: 3199cae (rel/v1.11.0.RC4)
Author: Jacob Barrett
Message: GEODE-7426: Fixes segfault in log message. (#545)


(cherry picked from commit 55da853760c200c53568fe2e6549c912ec26cc27)

View the changeset: 
https://github.com/apache/geode-native/compare/rel/v1.11.0.RC4

View the full build log and details: 
https://travis-ci.org/apache/geode-native/builds/627935150?utm_medium=notification&utm_source=email

--

You can unsubscribe from build emails from the apache/geode-native repository 
going to 
https://travis-ci.org/account/preferences/unsubscribe?repository=11948127&utm_medium=notification&utm_source=email.
Or unsubscribe from *all* email updating your settings at 
https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification&utm_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.

Re: [DISCUSS] `status locator` command fail when locator's ssl is turned on

[Jens Deppe]

I think Jinmei is also referring to the situation where you might *not* already
be connected but want to execute something like this:

gfsh>status locator --host=fooish --port=10334


In this case it would fail as well. Seems like we'd either need to remove
those options so you *always* need to 'connect' if you want to access a
remote system, or we need to add an *optional* --security-properties-file
option that would only be necessary if the shell isn't already connected.

--Jens




On Fri, Dec 20, 2019 at 3:15 PM Ernest Burghardt 
wrote:

> I like the approach Jens is suggesting, seems intuitive to me
>
> On Thu, Dec 19, 2019 at 6:31 PM Jens Deppe  wrote:
>
> > So it seems that the situation is something like this where I'm able to
> > make the initial connection but then retrieving status fails:
> >
> > gfsh>connect --security-properties-file=../security.properties
> >
> > gfsh>status locator --name=locator1
> > Server version response invalid: This could be the result of trying to
> > connect a non-SSL-enabled client to an SSL-enabled locator.
> >
> >
> > It would seem very weird if I have to provide additional connection
> params
> > to the 'status' command if I've already provided them as part of the
> > 'connect'. Could we not stash the connection properties (in the Gfsh
> > instance object) for subsequent usage?
> >
> > --Jens
> >
> > On Wed, Dec 18, 2019 at 9:04 AM Jinmei Liao  wrote:
> >
> > > "status locator" command is broken on ssl enabled locators ever since
> we
> > > fixed a bug that leaked the connection properties from one tcp socket
> > > connection to another. Before that it would just magically work if we
> > have
> > > previously made a successfully tcp connection to that same locator, now
> > we
> > > are either required to find a way to specify the ssl properties when
> > > running the `status locator` command or change what we want to report
> > back
> > > to the user.
> > >
> > > Here is what's happening now when we issue the command:
> > >
> > >1. it needs to retrieve two sets of info from locator: general info
> > like
> > >(pid, working dir, status, jvm args etc) and whether cluster
> > > configuration
> > >service is running or not.
> > >2. when locator’s ssl is on, the retrieval of the cluster
> > configuration
> > >info will always fail since it’s using a tcp connection to get that
> > info
> > >and we currently don’t have the ssl security properties to connect.
> > >3. when locator’s ssl is on, the retrieval of the general info will
> > >mostly succeed except when user is only providing a host and port,
> > > there we
> > >would also need the ssl security properties in order to create an
> ssl
> > >socket.
> > >
> > >
> > > So we wither need to add an option for user to specify a
> > > `--security-properties-file` option to include all the ssl connection
> > > properties or change the behavior not to report cluster config status
> or
> > > not allowing host/port combination. (I am not here long enough to
> > > understand what users would use this command for, so this is just a
> > random
> > > suggestions).
> > >
> > > Comments/thoughts?
> > >
> > > --
> > > Cheers
> > >
> > > Jinmei
> > >
> >
>

Re: [DISCUSS] Proposal to require linear commit history on develop

[Owen Nichols]

Bruce, you’re right, there doesn’t seem to be consensus, and a PR was
premature. I was reminded this week that voting is a “last resort”, not the
way to build consensus.  I have withdrawn the PR.

On Fri, Dec 20, 2019 at 10:18 AM Ernest Burghardt 
wrote:

> I'm a proponent of using squash-and-merge, and once a person has chosen
> this option once it comes up by default afterwards...
>
> Owen,  I don't think you have consensus to put forth this PR, there are -1s
> above... (early voting)
>
> maybe we'll be better off socializing the norm of squash-and-merge and
> gaining a natural consensus that way...
>
> On Fri, Dec 20, 2019 at 10:07 AM Owen Nichols  wrote:
>
> > The proposed action manifests as a commit to the Geode git repository, so
> > a PR is an acceptable vehicle for voting in this case.
> >
> > > On Dec 20, 2019, at 9:38 AM, Bruce Schuchardt 
> > wrote:
> > >
> > > I see a lot of plus-ones and a "voting deadline" on this DISCUSS thread
> > and a request to "vote" using a PR.  This all seems out of order to me.
> > Our votes are supposed to be on the email list, aren't they? and I
> haven't
> > seen a VOTE request.
> > >
> > > On 12/20/19 9:33 AM, Nabarun Nag wrote:
> > >> +1
> > >>
> > >> On Fri, Dec 20, 2019 at 9:25 AM Owen Nichols 
> > wrote:
> > >>
> > >>> Based on the feedback so far, I will amend the proposal to drop item
> > 2).
> > >>> Therefore, the current ability to create merge commits using
> > command-line
> > >>> git will remain available.
> > >>>
> > >>> The proposal as amended is now:
> >  Change GitHub settings to make "Squash and merge" the default
> >  (by removing “Create a merge commit” option).
> > 
> >  Update the PR template to change the text "Is your initial
> > contribution
> >  a single, squashed commit” to “Is your initial contribution squashed
> > for
> >  ease of review (e.g. a single commit, or a ‘refactoring’ commit
> plus a
> >  ‘fix’ commit)"
> > >>>
> > >>> As Naba suggested, we can try it, and if we don’t like it, it’s
> simple
> > to
> > >>> revert.
> > >>>
> > >>> I’ve create a PR for the proposed change here:
> > >>> https://github.com/apache/geode/pull/4513
> > >>>
> > >>> Please use the PR to vote for against this proposal.  It will not be
> > >>> merged before the VOTING DEADLINE of DEC 31 (if no -1’s at that time)
> > >>>
> >  On Dec 20, 2019, at 8:31 AM, Ju@N  wrote:
> > 
> >  +1
> > 
> >  On Fri 20 Dec 2019 at 16:18, Owen Nichols 
> > wrote:
> > 
> > > Hi Bruce, this proposal will not waste a single second of your
> > time.  It
> > > just prevents people from accidentally pressing a button that we
> have
> > > already agreed should never be pressed, but because we never
> > configured
> > >>> our
> > > GitHub to match our stated policy, is currently the default.
> > >
> > > However, it will save a lot of time and frustration for anyone
> > needing
> > >>> to
> > > bisect failures, revert, or cherry-pick changes, which has merit
> > even if
> > > you personally never do any of those three things.
> > >
> > > Please start a separate thread if you would like to revisit the
> > >>> community
> > > decision to require passing PR checks.
> > >
> > >> On Dec 20, 2019, at 7:49 AM, Bruce Schuchardt <
> > bschucha...@pivotal.io>
> > > wrote:
> > >> I agree with Jake.  I would go further by saying that I see very
> > little
> > > merit in this proposal.  I think we're getting more and more
> > >>> bureaucratic
> > > in our process and that it stifles productivity.  I was recently
> > forced
> > >>> to
> > > spend three days fixing tests in which I had changed an import
> > statement
> > > before they would pass stress testing.  I'm glad the tests now pass
> > > reliably but I was very frustrated by the process.
> > >> On 12/19/19 4:49 PM, Jacob Barrett wrote:
> > >>> I’m in agreement with Dan. Changes to the infrastructure to flat
> > out
> > > prevent things that should be self policing is annoying. This PR
> > review
> > > lock we have had already cost us valuable time waiting for PR
> > pipelines
> > >>> to
> > > pass that have no relevance to the commit, like CI work: I’d hat to
> > see
> > >>> yet
> > > another process enforced that Kees us from getting work done when
> > >>> necessary.
> > >>> -Jake
> > >>>
> > >>>
> >  On Dec 19, 2019, at 4:43 PM, Dan Smith 
> wrote:
> > 
> >  -1 to (1) and (2).
> > 
> >  I think merge commits are appropriate in certain circumstances,
> > so I
> > > don't
> >  think we should make a blanket restriction. In fact I think our
> > >>> release
> >  process involves some merges.
> > 
> >  I think setting standards on what is reasonable to be an
> > individual
> > > commit
> >  will do a lot more to clean up our history than blocking merges.
> > We'd
> >  rather not see