Review #4537

[Anthony Baker]

I’m looking for a +1 (or other feedback) on 
https://github.com/apache/geode/pull/4537 if anyone has cycles today.  Thanks!

Anthony

Errored: apache/geode-examples#397 (release/1.11.0 - eed34d3)

[Travis CI]

Build Update for apache/geode-examples
-

Build: #397
Status: Errored

Duration: 20 secs
Commit: eed34d3 (release/1.11.0)
Author: Mark Hanson
Message: Revert "temporarily point to staging repo for CI purposes"

View the changeset: 
https://github.com/apache/geode-examples/compare/8408797955fe...eed34d3f5a4f

View the full build log and details: 
https://travis-ci.org/apache/geode-examples/builds/631367654?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.

[ANNOUNCE] Apache Geode 1.11.0

[Mark Hanson]

The Apache Geode community is pleased to announce the availability of
Apache Geode 1.11.0.

Apache Geode is a data management platform that provides a database-like
consistency model, reliable transaction processing and a shared-nothing
architecture to maintain very low latency performance with high concurrency
processing.

Geode 1.11.0 contains a number of improvements and bug fixes.

For the full list of changes please review the release
notes:https://cwiki.apache.org/confluence/display/GEODE/
Release+Notes#ReleaseNotes-1.11.0

The release artifacts can be downloaded from the project
website:http://geode.apache.org/releases/

The release documentation is available
at:http://geode.apache.org/docs/guide/111/about_geode.html

We would like to thank all the contributors that made the release possible.
Regards,
Mark Hanson on behalf of the Apache Geode team

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

[Kirk Lund]

I'm happy to file multiple PRs when I need to merge multiple commits to
develop.

On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:

> This change to disable all but squash-merge would be really easy to
> revert. How about we try it for a while and see? If people decide it is
> really limiting them, we can change it back. Let’s do it for 1 month and
> see how it goes. Does that sound reasonable?
>
> Thanks,
> Mark
>
> > On Dec 30, 2019, at 5:25 PM, Owen Nichols  wrote:
> >
> > Given that we adopted <
> https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E>
> and still wish to continue <
> https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005f3ade0be777e679eafe95db@%3Cdev.geode.apache.org%3E>
> having branch protection rules to ensure every commit landing in develop
> has passed the required PR checks, it seems like that decision alone
> mandates that we disable all merge mechanisms other than squash-and-merge.
> >
> > Allowing merge commits or rebase merges bypasses branch protection for
> all commits other than the final one in the merge or rebase set.  Given
> that we decided <
> https://lists.apache.org/thread.html/1ba19d9aeb206148c922afdd182ba322d6f128bbb83983f2f72a108e@%3Cdev.geode.apache.org%3E>
> that bypassing PR checks should never be allowed, keeping this loophole
> open seems untenable.
> >
> > This is not just hypothetical — this loophole is causing real problems.
> We now have commits on develop that don’t compile.  For example:
> > git checkout 19eee7821322a1301f16bdcd31fd3b8c872a41b6
> > ./gradlew devBuild
> > ...spotlessJava FAILED
> > We implemented branch protections to make this impossible, right?
> >
> > We can very easily close this loophole by disabling all but the
> Squash&Merge button for PRs.  This will not make more work for any
> developer.  If you want to get multiple commits onto develop, simply submit
> each as a separate PR — that is the only way to assure that required PR
> checks have passed for each.
> >
> > On the other hand, if we as a Geode community feel strongly that
> bypassing branch protection via merge commits and rebase commits is
> important to allow, why not also allow arbitrary overrides (or get rid of
> branch protection entirely)?
> >
> > -Owen
> >
> >> On Dec 20, 2019, at 12:31 PM, Blake Bender  wrote:
> >>
> >> 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 c

RFC is about to finish collecting feedback: Support for clear operation on partitioned region

[Xiaojian Zhou]

Hi,

We have published the RFC: "Support for clear operation on partitioned
region" for about 2 weeks. Thank the community to have given a lot of
valuable feedback.

https://cwiki.apache.org/confluence/display/GEODE/Support+for+clear+operation+on+partitioned+region

We have updated the RFC accordingly and answered all the questions.

The period of collecting feedback will finish by the end of this week.
Should you have questions and concerns, please add to the RFC within this
week.

Thank you.

Regards
Xiaojian Zhou (Gester)

[DISCUSS] What should we do with @Ignore tests?

[Mark Hanson]

Hi All,

As part of what I am doing to fix flaky tests, I periodically come across tests 
that are @Ignore’d. I am curious what we would like to do with them generally 
speaking. We could fix them, which would seem obvious, but we are struggling to 
fix flaky tests as it is.  We could delete them, but those tests were written 
for a reason (I hope).  Or we could leave them. This pollutes searches etc as 
inactive code requiring upkeep at least.

I don’t have an easy answer. Some have suggested deleting them. I tend to lean 
that direction, but I thought I would consult the community for a broader 
perspective.

Thanks,
Mark

Re: [DISCUSS] What should we do with @Ignore tests?

[Alexander Murmann]

Here are a few things that are true for me or I believe are true in general:

   - Our test suite is more flaky than we'd like it to be
   - I don't believe that adding more Unit tests that follow existing
   patterns buys us that much. I'd rather see something similar to what some
   folks are doing with Membership right now where we isolate the code and
   test it more systematically
   - We have other testing gaps: We have benchmarks 👏🎉, but we are still
   lacking coverage in that ares; our community is still lacking HA tests. I'd
   rather fill those than bring back old DUnit tests that are chosen somewhat
   at random.
   - I'd rather be deliberate about what tests we introduce than wholesale
   bring back a set of tests, since any of these re-introduced tests has a
   potential to be flaky. Let's make sure our tests carry their weight.
   - If we delete these tests, we can always go back to a SHA from today
   and bring them back at a later date
   - These tests have been ignored since a very long time and we've shipped
   without them and nobody has missed them enough to bring them back.

Given all the above, my vote is for less noise in our code, which means
deleting all ignored tests. If we want to keep them, I'd love to hear a
plan of action on how we bring them back. Having a bunch of dead code helps
nobody.

On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson  wrote:

> Hi All,
>
> As part of what I am doing to fix flaky tests, I periodically come across
> tests that are @Ignore’d. I am curious what we would like to do with them
> generally speaking. We could fix them, which would seem obvious, but we are
> struggling to fix flaky tests as it is.  We could delete them, but those
> tests were written for a reason (I hope).  Or we could leave them. This
> pollutes searches etc as inactive code requiring upkeep at least.
>
> I don’t have an easy answer. Some have suggested deleting them. I tend to
> lean that direction, but I thought I would consult the community for a
> broader perspective.
>
> Thanks,
> Mark

Re: [DISCUSS] What should we do with @Ignore tests?

[Jacob Barrett]

+1 to Alexander

> On Dec 31, 2019, at 2:07 PM, Alexander Murmann  wrote:
> 
> Here are a few things that are true for me or I believe are true in general:
> 
>   - Our test suite is more flaky than we'd like it to be
>   - I don't believe that adding more Unit tests that follow existing
>   patterns buys us that much. I'd rather see something similar to what some
>   folks are doing with Membership right now where we isolate the code and
>   test it more systematically
>   - We have other testing gaps: We have benchmarks 👏🎉, but we are still
>   lacking coverage in that ares; our community is still lacking HA tests. I'd
>   rather fill those than bring back old DUnit tests that are chosen somewhat
>   at random.
>   - I'd rather be deliberate about what tests we introduce than wholesale
>   bring back a set of tests, since any of these re-introduced tests has a
>   potential to be flaky. Let's make sure our tests carry their weight.
>   - If we delete these tests, we can always go back to a SHA from today
>   and bring them back at a later date
>   - These tests have been ignored since a very long time and we've shipped
>   without them and nobody has missed them enough to bring them back.
> 
> Given all the above, my vote is for less noise in our code, which means
> deleting all ignored tests. If we want to keep them, I'd love to hear a
> plan of action on how we bring them back. Having a bunch of dead code helps
> nobody.
> 
> On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson  wrote:
> 
>> Hi All,
>> 
>> As part of what I am doing to fix flaky tests, I periodically come across
>> tests that are @Ignore’d. I am curious what we would like to do with them
>> generally speaking. We could fix them, which would seem obvious, but we are
>> struggling to fix flaky tests as it is.  We could delete them, but those
>> tests were written for a reason (I hope).  Or we could leave them. This
>> pollutes searches etc as inactive code requiring upkeep at least.
>> 
>> I don’t have an easy answer. Some have suggested deleting them. I tend to
>> lean that direction, but I thought I would consult the community for a
>> broader perspective.
>> 
>> Thanks,
>> Mark

Re: [DISCUSS] What should we do with @Ignore tests?

[Bruce Schuchardt]

I agree with deleting @Ignored tests except for the few that have JIRA 
tickets open for them.  There are less than 1/2 dozen of these and we 
should consider keeping them since we have a way of tracking them.


On 12/31/19 2:07 PM, Alexander Murmann wrote:

Here are a few things that are true for me or I believe are true in general:

- Our test suite is more flaky than we'd like it to be
- I don't believe that adding more Unit tests that follow existing
patterns buys us that much. I'd rather see something similar to what some
folks are doing with Membership right now where we isolate the code and
test it more systematically
- We have other testing gaps: We have benchmarks 👏🎉, but we are still
lacking coverage in that ares; our community is still lacking HA tests. I'd
rather fill those than bring back old DUnit tests that are chosen somewhat
at random.
- I'd rather be deliberate about what tests we introduce than wholesale
bring back a set of tests, since any of these re-introduced tests has a
potential to be flaky. Let's make sure our tests carry their weight.
- If we delete these tests, we can always go back to a SHA from today
and bring them back at a later date
- These tests have been ignored since a very long time and we've shipped
without them and nobody has missed them enough to bring them back.

Given all the above, my vote is for less noise in our code, which means
deleting all ignored tests. If we want to keep them, I'd love to hear a
plan of action on how we bring them back. Having a bunch of dead code helps
nobody.

On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson  wrote:


Hi All,

As part of what I am doing to fix flaky tests, I periodically come across
tests that are @Ignore’d. I am curious what we would like to do with them
generally speaking. We could fix them, which would seem obvious, but we are
struggling to fix flaky tests as it is.  We could delete them, but those
tests were written for a reason (I hope).  Or we could leave them. This
pollutes searches etc as inactive code requiring upkeep at least.

I don’t have an easy answer. Some have suggested deleting them. I tend to
lean that direction, but I thought I would consult the community for a
broader perspective.

Thanks,
Mark

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

[Owen Nichols]

To recap, this proposal is now revised to remove 2 of the 3 merge options
from GitHub, leaving only Squash and Merge.  PR #4513
 serves as an exhibit of what is
proposed (it is not to be merged unless discussion leads to consensus and a
successful vote).  Please use the dev list (not the PR) for all discussion
(and voting, if we get that far).

Squash and merge is already used almost exclusively by the Geode community,
with any exceptions tending to be accidental more often than intentional.
However, some have raised the concern that implementing this restriction
could result in harm or wasted time.  Can someone give an example of a such
a scenario?

It seems there is a divide here between junior and senior community
members.  Newer committers appreciate additional guardrails to protect them
from accidentally doing the wrong thing, while those with more experience
want to be able to work unencumbered by restrictions of any kind.

Our welcome email to new committers states “We like to work on trust rather
than unnecessary constraints...Being a committer enables you to more easily
make changes without needing to go through the patch submission process”.
I can see this as an argument against this proposal (perhaps even an
argument against any form of branch protection).

In the scheme of things, this proposal makes very little difference. There
are still other ways to get non-compiling commits onto develop (e.g.
waiting a long time between running PR checks and merging to develop).
What’s more important is working well together as a community. So, perhaps
what’s best for the community is to encourage working on trust rather than
unnecessary constraints.

-Owen

On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:

I'm happy to file multiple PRs when I need to merge multiple commits to
develop.

On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:

This change to disable all but squash-merge would be really easy to
revert. How about we try it for a while and see? If people decide it is
really limiting them, we can change it back. Let’s do it for 1 month and
see how it goes. Does that sound reasonable?

Thanks,
Mark

On Dec 30, 2019, at 5:25 PM, Owen Nichols  wrote:

Given that we adopted <

https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E
>
and still wish to continue <
https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005f3ade0be777e679eafe95db@%3Cdev.geode.apache.org%3E
>
having branch protection rules to ensure every commit landing in develop
has passed the required PR checks, it seems like that decision alone
mandates that we disable all merge mechanisms other than squash-and-merge.


Allowing merge commits or rebase merges bypasses branch protection for

all commits other than the final one in the merge or rebase set.  Given
that we decided <
https://lists.apache.org/thread.html/1ba19d9aeb206148c922afdd182ba322d6f128bbb83983f2f72a108e@%3Cdev.geode.apache.org%3E
>
that bypassing PR checks should never be allowed, keeping this loophole
open seems untenable.


This is not just hypothetical — this loophole is causing real problems.

We now have commits on develop that don’t compile.  For example:

git checkout 19eee7821322a1301f16bdcd31fd3b8c872a41b6
./gradlew devBuild
...spotlessJava FAILED
We implemented branch protections to make this impossible, right?

We can very easily close this loophole by disabling all but the

Squash&Merge button for PRs.  This will not make more work for any
developer.  If you want to get multiple commits onto develop, simply submit
each as a separate PR — that is the only way to assure that required PR
checks have passed for each.


On the other hand, if we as a Geode community feel strongly that

bypassing branch protection via merge commits and rebase commits is
important to allow, why not also allow arbitrary overrides (or get rid of
branch protection entirely)?


-Owen

On Dec 20, 2019, at 12:31 PM, Blake Bender  wrote:

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

Re: [DISCUSS] What should we do with @Ignore tests?

[Aaron Lindsey]

I’m in favor of deleting all except the ones that have JIRA tickets open for 
them, like Bruce said.

Also going forward I’d like to see us not be checking in @Ignored tests — just 
delete them instead. If we need to get it back we have revision history. Just 
my two cents.

Aaron

> On Dec 31, 2019, at 2:53 PM, Bruce Schuchardt  wrote:
> 
> I agree with deleting @Ignored tests except for the few that have JIRA 
> tickets open for them.  There are less than 1/2 dozen of these and we should 
> consider keeping them since we have a way of tracking them.
> 
> On 12/31/19 2:07 PM, Alexander Murmann wrote:
>> Here are a few things that are true for me or I believe are true in general:
>> 
>>- Our test suite is more flaky than we'd like it to be
>>- I don't believe that adding more Unit tests that follow existing
>>patterns buys us that much. I'd rather see something similar to what some
>>folks are doing with Membership right now where we isolate the code and
>>test it more systematically
>>- We have other testing gaps: We have benchmarks 👏🎉, but we are still
>>lacking coverage in that ares; our community is still lacking HA tests. 
>> I'd
>>rather fill those than bring back old DUnit tests that are chosen somewhat
>>at random.
>>- I'd rather be deliberate about what tests we introduce than wholesale
>>bring back a set of tests, since any of these re-introduced tests has a
>>potential to be flaky. Let's make sure our tests carry their weight.
>>- If we delete these tests, we can always go back to a SHA from today
>>and bring them back at a later date
>>- These tests have been ignored since a very long time and we've shipped
>>without them and nobody has missed them enough to bring them back.
>> 
>> Given all the above, my vote is for less noise in our code, which means
>> deleting all ignored tests. If we want to keep them, I'd love to hear a
>> plan of action on how we bring them back. Having a bunch of dead code helps
>> nobody.
>> 
>> On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson  wrote:
>> 
>>> Hi All,
>>> 
>>> As part of what I am doing to fix flaky tests, I periodically come across
>>> tests that are @Ignore’d. I am curious what we would like to do with them
>>> generally speaking. We could fix them, which would seem obvious, but we are
>>> struggling to fix flaky tests as it is.  We could delete them, but those
>>> tests were written for a reason (I hope).  Or we could leave them. This
>>> pollutes searches etc as inactive code requiring upkeep at least.
>>> 
>>> I don’t have an easy answer. Some have suggested deleting them. I tend to
>>> lean that direction, but I thought I would consult the community for a
>>> broader perspective.
>>> 
>>> Thanks,
>>> Mark

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

[Aaron Lindsey]

-0.9

I’m not in favor of the revised proposal that disallows rebase-and-merge. Say I 
am working on a PR and I have a refactor commit and another commit which 
implements a new feature. I don’t want those commits to get squashed because 
that makes it hard to understand the diff. However, if I make those commits as 
two separate PRs then I am going to have to deal with conflicts.

I’m not sure when we made it a rule that every commit in develop had to compile 
and pass tests. I know we implemented a rule that all PRs had to pass certain 
checks, but I never thought that rule implied all commits had to pass those 
checks.

In general I just don’t see the problem with rebase-and-merge and this feels 
like an unnecessary restriction, but I will go with it if that’s what everyone 
wants to do.

Aaron

> On Dec 31, 2019, at 3:09 PM, Owen Nichols  wrote:
> 
> To recap, this proposal is now revised to remove 2 of the 3 merge options
> from GitHub, leaving only Squash and Merge.  PR #4513
>  serves as an exhibit of what is
> proposed (it is not to be merged unless discussion leads to consensus and a
> successful vote).  Please use the dev list (not the PR) for all discussion
> (and voting, if we get that far).
> 
> Squash and merge is already used almost exclusively by the Geode community,
> with any exceptions tending to be accidental more often than intentional.
> However, some have raised the concern that implementing this restriction
> could result in harm or wasted time.  Can someone give an example of a such
> a scenario?
> 
> It seems there is a divide here between junior and senior community
> members.  Newer committers appreciate additional guardrails to protect them
> from accidentally doing the wrong thing, while those with more experience
> want to be able to work unencumbered by restrictions of any kind.
> 
> Our welcome email to new committers states “We like to work on trust rather
> than unnecessary constraints...Being a committer enables you to more easily
> make changes without needing to go through the patch submission process”.
> I can see this as an argument against this proposal (perhaps even an
> argument against any form of branch protection).
> 
> In the scheme of things, this proposal makes very little difference. There
> are still other ways to get non-compiling commits onto develop (e.g.
> waiting a long time between running PR checks and merging to develop).
> What’s more important is working well together as a community. So, perhaps
> what’s best for the community is to encourage working on trust rather than
> unnecessary constraints.
> 
> -Owen
> 
> On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:
> 
> I'm happy to file multiple PRs when I need to merge multiple commits to
> develop.
> 
> On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:
> 
> This change to disable all but squash-merge would be really easy to
> revert. How about we try it for a while and see? If people decide it is
> really limiting them, we can change it back. Let’s do it for 1 month and
> see how it goes. Does that sound reasonable?
> 
> Thanks,
> Mark
> 
> On Dec 30, 2019, at 5:25 PM, Owen Nichols  wrote:
> 
> Given that we adopted <
> 
> https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E
>> 
> and still wish to continue <
> https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005f3ade0be777e679eafe95db@%3Cdev.geode.apache.org%3E
>> 
> having branch protection rules to ensure every commit landing in develop
> has passed the required PR checks, it seems like that decision alone
> mandates that we disable all merge mechanisms other than squash-and-merge.
> 
> 
> Allowing merge commits or rebase merges bypasses branch protection for
> 
> all commits other than the final one in the merge or rebase set.  Given
> that we decided <
> https://lists.apache.org/thread.html/1ba19d9aeb206148c922afdd182ba322d6f128bbb83983f2f72a108e@%3Cdev.geode.apache.org%3E
>> 
> that bypassing PR checks should never be allowed, keeping this loophole
> open seems untenable.
> 
> 
> This is not just hypothetical — this loophole is causing real problems.
> 
> We now have commits on develop that don’t compile.  For example:
> 
> git checkout 19eee7821322a1301f16bdcd31fd3b8c872a41b6
> ./gradlew devBuild
> ...spotlessJava FAILED
> We implemented branch protections to make this impossible, right?
> 
> We can very easily close this loophole by disabling all but the
> 
> Squash&Merge button for PRs.  This will not make more work for any
> developer.  If you want to get multiple commits onto develop, simply submit
> each as a separate PR — that is the only way to assure that required PR
> checks have passed for each.
> 
> 
> On the other hand, if we as a Geode community feel strongly that
> 
> bypassing branch protection via merge commits and rebase commits is
> important to allow, why not also allow arbitrary overrides (or 

Re: [DISCUSS] What should we do with @Ignore tests?

[Dan Smith]

Some of these test have been ignored for a long time. However, looking at
the history, I see we have ignored some tests as recently as in the last
month, for what seem like some questionable reasons.

I'm concerned that this could be a two step process to losing test coverage
- someone who things the test is still valuable but intends to fix it later
ignores it, and then someone else deletes it.

So what I would suggest is that if we are going to delete them, let's do it
in batches in get folks that have context on the code being tested to
review the changes. Make sense?

Also +1 to not ignoring any more tests - it would be nice to get down to 0
Ignored tests and enforce that!

-Dan

On Tue, Dec 31, 2019 at 4:52 PM Aaron Lindsey 
wrote:

> I’m in favor of deleting all except the ones that have JIRA tickets open
> for them, like Bruce said.
>
> Also going forward I’d like to see us not be checking in @Ignored tests —
> just delete them instead. If we need to get it back we have revision
> history. Just my two cents.
>
> Aaron
>
> > On Dec 31, 2019, at 2:53 PM, Bruce Schuchardt 
> wrote:
> >
> > I agree with deleting @Ignored tests except for the few that have JIRA
> tickets open for them.  There are less than 1/2 dozen of these and we
> should consider keeping them since we have a way of tracking them.
> >
> > On 12/31/19 2:07 PM, Alexander Murmann wrote:
> >> Here are a few things that are true for me or I believe are true in
> general:
> >>
> >>- Our test suite is more flaky than we'd like it to be
> >>- I don't believe that adding more Unit tests that follow existing
> >>patterns buys us that much. I'd rather see something similar to what
> some
> >>folks are doing with Membership right now where we isolate the code
> and
> >>test it more systematically
> >>- We have other testing gaps: We have benchmarks 👏🎉, but we are
> still
> >>lacking coverage in that ares; our community is still lacking HA
> tests. I'd
> >>rather fill those than bring back old DUnit tests that are chosen
> somewhat
> >>at random.
> >>- I'd rather be deliberate about what tests we introduce than
> wholesale
> >>bring back a set of tests, since any of these re-introduced tests
> has a
> >>potential to be flaky. Let's make sure our tests carry their weight.
> >>- If we delete these tests, we can always go back to a SHA from today
> >>and bring them back at a later date
> >>- These tests have been ignored since a very long time and we've
> shipped
> >>without them and nobody has missed them enough to bring them back.
> >>
> >> Given all the above, my vote is for less noise in our code, which means
> >> deleting all ignored tests. If we want to keep them, I'd love to hear a
> >> plan of action on how we bring them back. Having a bunch of dead code
> helps
> >> nobody.
> >>
> >> On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson  wrote:
> >>
> >>> Hi All,
> >>>
> >>> As part of what I am doing to fix flaky tests, I periodically come
> across
> >>> tests that are @Ignore’d. I am curious what we would like to do with
> them
> >>> generally speaking. We could fix them, which would seem obvious, but
> we are
> >>> struggling to fix flaky tests as it is.  We could delete them, but
> those
> >>> tests were written for a reason (I hope).  Or we could leave them. This
> >>> pollutes searches etc as inactive code requiring upkeep at least.
> >>>
> >>> I don’t have an easy answer. Some have suggested deleting them. I tend
> to
> >>> lean that direction, but I thought I would consult the community for a
> >>> broader perspective.
> >>>
> >>> Thanks,
> >>> Mark
>
>

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

[Owen Nichols]

Can you elaborate on why you would have to deal with conflicts if the
refactoring work was kept as a separate PR from the fix?

As with many git workflows, there’s an easy way and a hard way to manage
interdependent PRs (tl;dr only merge, never rebase!). I wonder if this
points out an opportunity for a “tips and tricks” page on the Geode wiki.

On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey 
wrote:

> -0.9
>
> I’m not in favor of the revised proposal that disallows rebase-and-merge.
> Say I am working on a PR and I have a refactor commit and another commit
> which implements a new feature. I don’t want those commits to get squashed
> because that makes it hard to understand the diff. However, if I make those
> commits as two separate PRs then I am going to have to deal with conflicts.
>
> I’m not sure when we made it a rule that every commit in develop had to
> compile and pass tests. I know we implemented a rule that all PRs had to
> pass certain checks, but I never thought that rule implied all commits had
> to pass those checks.
>
> In general I just don’t see the problem with rebase-and-merge and this
> feels like an unnecessary restriction, but I will go with it if that’s what
> everyone wants to do.
>
> Aaron
>
> > On Dec 31, 2019, at 3:09 PM, Owen Nichols  wrote:
> >
> > To recap, this proposal is now revised to remove 2 of the 3 merge options
> > from GitHub, leaving only Squash and Merge.  PR #4513
> >  serves as an exhibit of
> what is
> > proposed (it is not to be merged unless discussion leads to consensus
> and a
> > successful vote).  Please use the dev list (not the PR) for all
> discussion
> > (and voting, if we get that far).
> >
> > Squash and merge is already used almost exclusively by the Geode
> community,
> > with any exceptions tending to be accidental more often than intentional.
> > However, some have raised the concern that implementing this restriction
> > could result in harm or wasted time.  Can someone give an example of a
> such
> > a scenario?
> >
> > It seems there is a divide here between junior and senior community
> > members.  Newer committers appreciate additional guardrails to protect
> them
> > from accidentally doing the wrong thing, while those with more experience
> > want to be able to work unencumbered by restrictions of any kind.
> >
> > Our welcome email to new committers states “We like to work on trust
> rather
> > than unnecessary constraints...Being a committer enables you to more
> easily
> > make changes without needing to go through the patch submission process”.
> > I can see this as an argument against this proposal (perhaps even an
> > argument against any form of branch protection).
> >
> > In the scheme of things, this proposal makes very little difference.
> There
> > are still other ways to get non-compiling commits onto develop (e.g.
> > waiting a long time between running PR checks and merging to develop).
> > What’s more important is working well together as a community. So,
> perhaps
> > what’s best for the community is to encourage working on trust rather
> than
> > unnecessary constraints.
> >
> > -Owen
> >
> > On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:
> >
> > I'm happy to file multiple PRs when I need to merge multiple commits to
> > develop.
> >
> > On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:
> >
> > This change to disable all but squash-merge would be really easy to
> > revert. How about we try it for a while and see? If people decide it is
> > really limiting them, we can change it back. Let’s do it for 1 month and
> > see how it goes. Does that sound reasonable?
> >
> > Thanks,
> > Mark
> >
> > On Dec 30, 2019, at 5:25 PM, Owen Nichols  wrote:
> >
> > Given that we adopted <
> >
> >
> https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E
> >>
> > and still wish to continue <
> >
> https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005f3ade0be777e679eafe95db@%3Cdev.geode.apache.org%3E
> >>
> > having branch protection rules to ensure every commit landing in develop
> > has passed the required PR checks, it seems like that decision alone
> > mandates that we disable all merge mechanisms other than
> squash-and-merge.
> >
> >
> > Allowing merge commits or rebase merges bypasses branch protection for
> >
> > all commits other than the final one in the merge or rebase set.  Given
> > that we decided <
> >
> https://lists.apache.org/thread.html/1ba19d9aeb206148c922afdd182ba322d6f128bbb83983f2f72a108e@%3Cdev.geode.apache.org%3E
> >>
> > that bypassing PR checks should never be allowed, keeping this loophole
> > open seems untenable.
> >
> >
> > This is not just hypothetical — this loophole is causing real problems.
> >
> > We now have commits on develop that don’t compile.  For example:
> >
> > git checkout 19eee7821322a1301f16bdcd31fd3b8c872a41b6
> > ./gradlew devBuild
> > ...spotlessJava FAILED
> > We im

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

[Aaron Lindsey]

Suppose I’m refactoring the same classes I’m touching for the feature. I do the 
refactoring on one branch and submit a PR for that. Then I implement the 
feature on another branch which is based on develop and does not have the 
refactoring changes since those are not merged yet. I’ll have to resolve 
conflicts on the second PR that I merge.

Having interdependent PRs where one PR branch is based on another PR branch is 
not something I’ve tried. That requires more overhead in having to create more 
PRs and branches. And if you have N interdependent PRs, you have to do N-1 
merges each time a PR is merged to develop (to update the other branches). It 
could be done, but is it worth the hassle?

One more point about rebase-and-merge is that it seems like this would make 
bisecting a failure easier since there would be smaller commits.

Aaron

> On Dec 31, 2019, at 5:41 PM, Owen Nichols  wrote:
> 
> Can you elaborate on why you would have to deal with conflicts if the
> refactoring work was kept as a separate PR from the fix?
> 
> As with many git workflows, there’s an easy way and a hard way to manage
> interdependent PRs (tl;dr only merge, never rebase!). I wonder if this
> points out an opportunity for a “tips and tricks” page on the Geode wiki.
> 
> On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey 
> wrote:
> 
>> -0.9
>> 
>> I’m not in favor of the revised proposal that disallows rebase-and-merge.
>> Say I am working on a PR and I have a refactor commit and another commit
>> which implements a new feature. I don’t want those commits to get squashed
>> because that makes it hard to understand the diff. However, if I make those
>> commits as two separate PRs then I am going to have to deal with conflicts.
>> 
>> I’m not sure when we made it a rule that every commit in develop had to
>> compile and pass tests. I know we implemented a rule that all PRs had to
>> pass certain checks, but I never thought that rule implied all commits had
>> to pass those checks.
>> 
>> In general I just don’t see the problem with rebase-and-merge and this
>> feels like an unnecessary restriction, but I will go with it if that’s what
>> everyone wants to do.
>> 
>> Aaron
>> 
>>> On Dec 31, 2019, at 3:09 PM, Owen Nichols  wrote:
>>> 
>>> To recap, this proposal is now revised to remove 2 of the 3 merge options
>>> from GitHub, leaving only Squash and Merge.  PR #4513
>>>  serves as an exhibit of
>> what is
>>> proposed (it is not to be merged unless discussion leads to consensus
>> and a
>>> successful vote).  Please use the dev list (not the PR) for all
>> discussion
>>> (and voting, if we get that far).
>>> 
>>> Squash and merge is already used almost exclusively by the Geode
>> community,
>>> with any exceptions tending to be accidental more often than intentional.
>>> However, some have raised the concern that implementing this restriction
>>> could result in harm or wasted time.  Can someone give an example of a
>> such
>>> a scenario?
>>> 
>>> It seems there is a divide here between junior and senior community
>>> members.  Newer committers appreciate additional guardrails to protect
>> them
>>> from accidentally doing the wrong thing, while those with more experience
>>> want to be able to work unencumbered by restrictions of any kind.
>>> 
>>> Our welcome email to new committers states “We like to work on trust
>> rather
>>> than unnecessary constraints...Being a committer enables you to more
>> easily
>>> make changes without needing to go through the patch submission process”.
>>> I can see this as an argument against this proposal (perhaps even an
>>> argument against any form of branch protection).
>>> 
>>> In the scheme of things, this proposal makes very little difference.
>> There
>>> are still other ways to get non-compiling commits onto develop (e.g.
>>> waiting a long time between running PR checks and merging to develop).
>>> What’s more important is working well together as a community. So,
>> perhaps
>>> what’s best for the community is to encourage working on trust rather
>> than
>>> unnecessary constraints.
>>> 
>>> -Owen
>>> 
>>> On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:
>>> 
>>> I'm happy to file multiple PRs when I need to merge multiple commits to
>>> develop.
>>> 
>>> On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:
>>> 
>>> This change to disable all but squash-merge would be really easy to
>>> revert. How about we try it for a while and see? If people decide it is
>>> really limiting them, we can change it back. Let’s do it for 1 month and
>>> see how it goes. Does that sound reasonable?
>>> 
>>> Thanks,
>>> Mark
>>> 
>>> On Dec 30, 2019, at 5:25 PM, Owen Nichols  wrote:
>>> 
>>> Given that we adopted <
>>> 
>>> 
>> https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E
 
>>> and still wish to continue <
>>> 
>> https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005

Re: [DISCUSS] What should we do with @Ignore tests?

[Nabarun Nag]

+1 to Dan's suggestions.

- Remove in batches.
- Send review requests for those PRs to relevant committers (authors of
those tests etc.)
- A brief explanation on why these tests are being deleted, and there is no
loss of test coverage as it is covered by these other tests (or some other
reason).

Regards
Nabarun Nag

On Tue, Dec 31, 2019 at 5:32 PM Dan Smith  wrote:

> Some of these test have been ignored for a long time. However, looking at
> the history, I see we have ignored some tests as recently as in the last
> month, for what seem like some questionable reasons.
>
> I'm concerned that this could be a two step process to losing test coverage
> - someone who things the test is still valuable but intends to fix it later
> ignores it, and then someone else deletes it.
>
> So what I would suggest is that if we are going to delete them, let's do it
> in batches in get folks that have context on the code being tested to
> review the changes. Make sense?
>
> Also +1 to not ignoring any more tests - it would be nice to get down to 0
> Ignored tests and enforce that!
>
> -Dan
>
> On Tue, Dec 31, 2019 at 4:52 PM Aaron Lindsey 
> wrote:
>
> > I’m in favor of deleting all except the ones that have JIRA tickets open
> > for them, like Bruce said.
> >
> > Also going forward I’d like to see us not be checking in @Ignored tests —
> > just delete them instead. If we need to get it back we have revision
> > history. Just my two cents.
> >
> > Aaron
> >
> > > On Dec 31, 2019, at 2:53 PM, Bruce Schuchardt 
> > wrote:
> > >
> > > I agree with deleting @Ignored tests except for the few that have JIRA
> > tickets open for them.  There are less than 1/2 dozen of these and we
> > should consider keeping them since we have a way of tracking them.
> > >
> > > On 12/31/19 2:07 PM, Alexander Murmann wrote:
> > >> Here are a few things that are true for me or I believe are true in
> > general:
> > >>
> > >>- Our test suite is more flaky than we'd like it to be
> > >>- I don't believe that adding more Unit tests that follow existing
> > >>patterns buys us that much. I'd rather see something similar to
> what
> > some
> > >>folks are doing with Membership right now where we isolate the code
> > and
> > >>test it more systematically
> > >>- We have other testing gaps: We have benchmarks 👏🎉, but we are
> > still
> > >>lacking coverage in that ares; our community is still lacking HA
> > tests. I'd
> > >>rather fill those than bring back old DUnit tests that are chosen
> > somewhat
> > >>at random.
> > >>- I'd rather be deliberate about what tests we introduce than
> > wholesale
> > >>bring back a set of tests, since any of these re-introduced tests
> > has a
> > >>potential to be flaky. Let's make sure our tests carry their
> weight.
> > >>- If we delete these tests, we can always go back to a SHA from
> today
> > >>and bring them back at a later date
> > >>- These tests have been ignored since a very long time and we've
> > shipped
> > >>without them and nobody has missed them enough to bring them back.
> > >>
> > >> Given all the above, my vote is for less noise in our code, which
> means
> > >> deleting all ignored tests. If we want to keep them, I'd love to hear
> a
> > >> plan of action on how we bring them back. Having a bunch of dead code
> > helps
> > >> nobody.
> > >>
> > >> On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson 
> wrote:
> > >>
> > >>> Hi All,
> > >>>
> > >>> As part of what I am doing to fix flaky tests, I periodically come
> > across
> > >>> tests that are @Ignore’d. I am curious what we would like to do with
> > them
> > >>> generally speaking. We could fix them, which would seem obvious, but
> > we are
> > >>> struggling to fix flaky tests as it is.  We could delete them, but
> > those
> > >>> tests were written for a reason (I hope).  Or we could leave them.
> This
> > >>> pollutes searches etc as inactive code requiring upkeep at least.
> > >>>
> > >>> I don’t have an easy answer. Some have suggested deleting them. I
> tend
> > to
> > >>> lean that direction, but I thought I would consult the community for
> a
> > >>> broader perspective.
> > >>>
> > >>> Thanks,
> > >>> Mark
> >
> >
>

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

[Nabarun Nag]

Aaron ,

Is it not the case currently? If I am working on a feature modifying class
X and another developer makes some refactoring changes on class X and
pushes it to develop, won't I have to resolve the merge commits anyway.


Regards
Naba


On Tue, Dec 31, 2019 at 6:01 PM Aaron Lindsey 
wrote:

> Suppose I’m refactoring the same classes I’m touching for the feature. I
> do the refactoring on one branch and submit a PR for that. Then I implement
> the feature on another branch which is based on develop and does not have
> the refactoring changes since those are not merged yet. I’ll have to
> resolve conflicts on the second PR that I merge.
>
> Having interdependent PRs where one PR branch is based on another PR
> branch is not something I’ve tried. That requires more overhead in having
> to create more PRs and branches. And if you have N interdependent PRs, you
> have to do N-1 merges each time a PR is merged to develop (to update the
> other branches). It could be done, but is it worth the hassle?
>
> One more point about rebase-and-merge is that it seems like this would
> make bisecting a failure easier since there would be smaller commits.
>
> Aaron
>
> > On Dec 31, 2019, at 5:41 PM, Owen Nichols  wrote:
> >
> > Can you elaborate on why you would have to deal with conflicts if the
> > refactoring work was kept as a separate PR from the fix?
> >
> > As with many git workflows, there’s an easy way and a hard way to manage
> > interdependent PRs (tl;dr only merge, never rebase!). I wonder if this
> > points out an opportunity for a “tips and tricks” page on the Geode wiki.
> >
> > On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey 
> > wrote:
> >
> >> -0.9
> >>
> >> I’m not in favor of the revised proposal that disallows
> rebase-and-merge.
> >> Say I am working on a PR and I have a refactor commit and another commit
> >> which implements a new feature. I don’t want those commits to get
> squashed
> >> because that makes it hard to understand the diff. However, if I make
> those
> >> commits as two separate PRs then I am going to have to deal with
> conflicts.
> >>
> >> I’m not sure when we made it a rule that every commit in develop had to
> >> compile and pass tests. I know we implemented a rule that all PRs had to
> >> pass certain checks, but I never thought that rule implied all commits
> had
> >> to pass those checks.
> >>
> >> In general I just don’t see the problem with rebase-and-merge and this
> >> feels like an unnecessary restriction, but I will go with it if that’s
> what
> >> everyone wants to do.
> >>
> >> Aaron
> >>
> >>> On Dec 31, 2019, at 3:09 PM, Owen Nichols  wrote:
> >>>
> >>> To recap, this proposal is now revised to remove 2 of the 3 merge
> options
> >>> from GitHub, leaving only Squash and Merge.  PR #4513
> >>>  serves as an exhibit of
> >> what is
> >>> proposed (it is not to be merged unless discussion leads to consensus
> >> and a
> >>> successful vote).  Please use the dev list (not the PR) for all
> >> discussion
> >>> (and voting, if we get that far).
> >>>
> >>> Squash and merge is already used almost exclusively by the Geode
> >> community,
> >>> with any exceptions tending to be accidental more often than
> intentional.
> >>> However, some have raised the concern that implementing this
> restriction
> >>> could result in harm or wasted time.  Can someone give an example of a
> >> such
> >>> a scenario?
> >>>
> >>> It seems there is a divide here between junior and senior community
> >>> members.  Newer committers appreciate additional guardrails to protect
> >> them
> >>> from accidentally doing the wrong thing, while those with more
> experience
> >>> want to be able to work unencumbered by restrictions of any kind.
> >>>
> >>> Our welcome email to new committers states “We like to work on trust
> >> rather
> >>> than unnecessary constraints...Being a committer enables you to more
> >> easily
> >>> make changes without needing to go through the patch submission
> process”.
> >>> I can see this as an argument against this proposal (perhaps even an
> >>> argument against any form of branch protection).
> >>>
> >>> In the scheme of things, this proposal makes very little difference.
> >> There
> >>> are still other ways to get non-compiling commits onto develop (e.g.
> >>> waiting a long time between running PR checks and merging to develop).
> >>> What’s more important is working well together as a community. So,
> >> perhaps
> >>> what’s best for the community is to encourage working on trust rather
> >> than
> >>> unnecessary constraints.
> >>>
> >>> -Owen
> >>>
> >>> On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:
> >>>
> >>> I'm happy to file multiple PRs when I need to merge multiple commits to
> >>> develop.
> >>>
> >>> On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson 
> wrote:
> >>>
> >>> This change to disable all but squash-merge would be really easy to
> >>> revert. How about we try it for a while and see? If people decide it is

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

[Jacob Barrett]

-1 on removing rebase option. I use it frequently to void squashing properly 
independent commits, like a cleanup or refactor and then a fix commit.

-Jake


> On Dec 31, 2019, at 3:10 PM, Owen Nichols  wrote:
> 
> To recap, this proposal is now revised to remove 2 of the 3 merge options
> from GitHub, leaving only Squash and Merge.  PR #4513
>  serves as an exhibit of what is
> proposed (it is not to be merged unless discussion leads to consensus and a
> successful vote).  Please use the dev list (not the PR) for all discussion
> (and voting, if we get that far).
> 
> Squash and merge is already used almost exclusively by the Geode community,
> with any exceptions tending to be accidental more often than intentional.
> However, some have raised the concern that implementing this restriction
> could result in harm or wasted time.  Can someone give an example of a such
> a scenario?
> 
> It seems there is a divide here between junior and senior community
> members.  Newer committers appreciate additional guardrails to protect them
> from accidentally doing the wrong thing, while those with more experience
> want to be able to work unencumbered by restrictions of any kind.
> 
> Our welcome email to new committers states “We like to work on trust rather
> than unnecessary constraints...Being a committer enables you to more easily
> make changes without needing to go through the patch submission process”.
> I can see this as an argument against this proposal (perhaps even an
> argument against any form of branch protection).
> 
> In the scheme of things, this proposal makes very little difference. There
> are still other ways to get non-compiling commits onto develop (e.g.
> waiting a long time between running PR checks and merging to develop).
> What’s more important is working well together as a community. So, perhaps
> what’s best for the community is to encourage working on trust rather than
> unnecessary constraints.
> 
> -Owen
> 
> On Dec 31, 2019, at 10:56 AM, Kirk Lund  wrote:
> 
> I'm happy to file multiple PRs when I need to merge multiple commits to
> develop.
> 
> On Mon, Dec 30, 2019 at 5:45 PM Mark Hanson  wrote:
> 
> This change to disable all but squash-merge would be really easy to
> revert. How about we try it for a while and see? If people decide it is
> really limiting them, we can change it back. Let’s do it for 1 month and
> see how it goes. Does that sound reasonable?
> 
> Thanks,
> Mark
> 
> On Dec 30, 2019, at 5:25 PM, Owen Nichols  wrote:
> 
> Given that we adopted <
> 
> https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E
>> 
> and still wish to continue <
> https://lists.apache.org/thread.html/8795697c1ad57068c053b48b4b1845005f3ade0be777e679eafe95db@%3Cdev.geode.apache.org%3E
>> 
> having branch protection rules to ensure every commit landing in develop
> has passed the required PR checks, it seems like that decision alone
> mandates that we disable all merge mechanisms other than squash-and-merge.
> 
> 
> Allowing merge commits or rebase merges bypasses branch protection for
> 
> all commits other than the final one in the merge or rebase set.  Given
> that we decided <
> https://lists.apache.org/thread.html/1ba19d9aeb206148c922afdd182ba322d6f128bbb83983f2f72a108e@%3Cdev.geode.apache.org%3E
>> 
> that bypassing PR checks should never be allowed, keeping this loophole
> open seems untenable.
> 
> 
> This is not just hypothetical — this loophole is causing real problems.
> 
> We now have commits on develop that don’t compile.  For example:
> 
> git checkout 19eee7821322a1301f16bdcd31fd3b8c872a41b6
> ./gradlew devBuild
> ...spotlessJava FAILED
> We implemented branch protections to make this impossible, right?
> 
> We can very easily close this loophole by disabling all but the
> 
> Squash&Merge button for PRs.  This will not make more work for any
> developer.  If you want to get multiple commits onto develop, simply submit
> each as a separate PR — that is the only way to assure that required PR
> checks have passed for each.
> 
> 
> On the other hand, if we as a Geode community feel strongly that
> 
> bypassing branch protection via merge commits and rebase commits is
> important to allow, why not also allow arbitrary overrides (or get rid of
> branch protection entirely)?
> 
> 
> -Owen
> 
> On Dec 20, 2019, at 12:31 PM, Blake Bender  wrote:
> 
> 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 el

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

[Dan Smith]

I also tend to do the same workflow Aaron described - make a refactoring
change to support a feature followed by the actual change I want to make.
It makes the history a lot clearer because refactoring tends to touch a lot
of files, so it's nice to have those changes clearly marked as a
refactoring that should not change behavior.

It's possible to do this as to separate PRs, but that drags out the process
because you have to get the first in merged before you create the second.
That encourages bunching changes in a single squashed commit, which makes
git blame less useful.


-Dan

On Tue, Dec 31, 2019, 6:36 PM Nabarun Nag  wrote:

> Aaron ,
>
> Is it not the case currently? If I am working on a feature modifying class
> X and another developer makes some refactoring changes on class X and
> pushes it to develop, won't I have to resolve the merge commits anyway.
>
>
> Regards
> Naba
>
>
> On Tue, Dec 31, 2019 at 6:01 PM Aaron Lindsey 
> wrote:
>
> > Suppose I’m refactoring the same classes I’m touching for the feature. I
> > do the refactoring on one branch and submit a PR for that. Then I
> implement
> > the feature on another branch which is based on develop and does not have
> > the refactoring changes since those are not merged yet. I’ll have to
> > resolve conflicts on the second PR that I merge.
> >
> > Having interdependent PRs where one PR branch is based on another PR
> > branch is not something I’ve tried. That requires more overhead in having
> > to create more PRs and branches. And if you have N interdependent PRs,
> you
> > have to do N-1 merges each time a PR is merged to develop (to update the
> > other branches). It could be done, but is it worth the hassle?
> >
> > One more point about rebase-and-merge is that it seems like this would
> > make bisecting a failure easier since there would be smaller commits.
> >
> > Aaron
> >
> > > On Dec 31, 2019, at 5:41 PM, Owen Nichols  wrote:
> > >
> > > Can you elaborate on why you would have to deal with conflicts if the
> > > refactoring work was kept as a separate PR from the fix?
> > >
> > > As with many git workflows, there’s an easy way and a hard way to
> manage
> > > interdependent PRs (tl;dr only merge, never rebase!). I wonder if this
> > > points out an opportunity for a “tips and tricks” page on the Geode
> wiki.
> > >
> > > On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey  >
> > > wrote:
> > >
> > >> -0.9
> > >>
> > >> I’m not in favor of the revised proposal that disallows
> > rebase-and-merge.
> > >> Say I am working on a PR and I have a refactor commit and another
> commit
> > >> which implements a new feature. I don’t want those commits to get
> > squashed
> > >> because that makes it hard to understand the diff. However, if I make
> > those
> > >> commits as two separate PRs then I am going to have to deal with
> > conflicts.
> > >>
> > >> I’m not sure when we made it a rule that every commit in develop had
> to
> > >> compile and pass tests. I know we implemented a rule that all PRs had
> to
> > >> pass certain checks, but I never thought that rule implied all commits
> > had
> > >> to pass those checks.
> > >>
> > >> In general I just don’t see the problem with rebase-and-merge and this
> > >> feels like an unnecessary restriction, but I will go with it if that’s
> > what
> > >> everyone wants to do.
> > >>
> > >> Aaron
> > >>
> > >>> On Dec 31, 2019, at 3:09 PM, Owen Nichols 
> wrote:
> > >>>
> > >>> To recap, this proposal is now revised to remove 2 of the 3 merge
> > options
> > >>> from GitHub, leaving only Squash and Merge.  PR #4513
> > >>>  serves as an exhibit of
> > >> what is
> > >>> proposed (it is not to be merged unless discussion leads to consensus
> > >> and a
> > >>> successful vote).  Please use the dev list (not the PR) for all
> > >> discussion
> > >>> (and voting, if we get that far).
> > >>>
> > >>> Squash and merge is already used almost exclusively by the Geode
> > >> community,
> > >>> with any exceptions tending to be accidental more often than
> > intentional.
> > >>> However, some have raised the concern that implementing this
> > restriction
> > >>> could result in harm or wasted time.  Can someone give an example of
> a
> > >> such
> > >>> a scenario?
> > >>>
> > >>> It seems there is a divide here between junior and senior community
> > >>> members.  Newer committers appreciate additional guardrails to
> protect
> > >> them
> > >>> from accidentally doing the wrong thing, while those with more
> > experience
> > >>> want to be able to work unencumbered by restrictions of any kind.
> > >>>
> > >>> Our welcome email to new committers states “We like to work on trust
> > >> rather
> > >>> than unnecessary constraints...Being a committer enables you to more
> > >> easily
> > >>> make changes without needing to go through the patch submission
> > process”.
> > >>> I can see this as an argument against this proposal (perhaps even an
> > >>> argument against any form of

Re: [DISCUSS] What should we do with @Ignore tests?

[Mark Hanson]

Hi Naba,

While I think what you are suggesting sounds reasonable,  I think what you are 
proposing is a more painful process then leaving them in.  I am encountering 
maybe two of them at once when addressing a flaky test. If we want to do big 
bulk removes then  the burden of research becomes less likely to happen.  Just 
a thought.

Thanks,
Mark

Sent from my iPhone

> On Dec 31, 2019, at 6:31 PM, Nabarun Nag  wrote:
> 
> +1 to Dan's suggestions.
> 
> - Remove in batches.
> - Send review requests for those PRs to relevant committers (authors of
> those tests etc.)
> - A brief explanation on why these tests are being deleted, and there is no
> loss of test coverage as it is covered by these other tests (or some other
> reason).
> 
> Regards
> Nabarun Nag
> 
>> On Tue, Dec 31, 2019 at 5:32 PM Dan Smith  wrote:
>> 
>> Some of these test have been ignored for a long time. However, looking at
>> the history, I see we have ignored some tests as recently as in the last
>> month, for what seem like some questionable reasons.
>> 
>> I'm concerned that this could be a two step process to losing test coverage
>> - someone who things the test is still valuable but intends to fix it later
>> ignores it, and then someone else deletes it.
>> 
>> So what I would suggest is that if we are going to delete them, let's do it
>> in batches in get folks that have context on the code being tested to
>> review the changes. Make sense?
>> 
>> Also +1 to not ignoring any more tests - it would be nice to get down to 0
>> Ignored tests and enforce that!
>> 
>> -Dan
>> 
>> On Tue, Dec 31, 2019 at 4:52 PM Aaron Lindsey 
>> wrote:
>> 
>>> I’m in favor of deleting all except the ones that have JIRA tickets open
>>> for them, like Bruce said.
>>> 
>>> Also going forward I’d like to see us not be checking in @Ignored tests —
>>> just delete them instead. If we need to get it back we have revision
>>> history. Just my two cents.
>>> 
>>> Aaron
>>> 
 On Dec 31, 2019, at 2:53 PM, Bruce Schuchardt 
>>> wrote:
 
 I agree with deleting @Ignored tests except for the few that have JIRA
>>> tickets open for them.  There are less than 1/2 dozen of these and we
>>> should consider keeping them since we have a way of tracking them.
 
 On 12/31/19 2:07 PM, Alexander Murmann wrote:
> Here are a few things that are true for me or I believe are true in
>>> general:
> 
>   - Our test suite is more flaky than we'd like it to be
>   - I don't believe that adding more Unit tests that follow existing
>   patterns buys us that much. I'd rather see something similar to
>> what
>>> some
>   folks are doing with Membership right now where we isolate the code
>>> and
>   test it more systematically
>   - We have other testing gaps: We have benchmarks 👏🎉, but we are
>>> still
>   lacking coverage in that ares; our community is still lacking HA
>>> tests. I'd
>   rather fill those than bring back old DUnit tests that are chosen
>>> somewhat
>   at random.
>   - I'd rather be deliberate about what tests we introduce than
>>> wholesale
>   bring back a set of tests, since any of these re-introduced tests
>>> has a
>   potential to be flaky. Let's make sure our tests carry their
>> weight.
>   - If we delete these tests, we can always go back to a SHA from
>> today
>   and bring them back at a later date
>   - These tests have been ignored since a very long time and we've
>>> shipped
>   without them and nobody has missed them enough to bring them back.
> 
> Given all the above, my vote is for less noise in our code, which
>> means
> deleting all ignored tests. If we want to keep them, I'd love to hear
>> a
> plan of action on how we bring them back. Having a bunch of dead code
>>> helps
> nobody.
> 
> On Tue, Dec 31, 2019 at 1:50 PM Mark Hanson 
>> wrote:
> 
>> Hi All,
>> 
>> As part of what I am doing to fix flaky tests, I periodically come
>>> across
>> tests that are @Ignore’d. I am curious what we would like to do with
>>> them
>> generally speaking. We could fix them, which would seem obvious, but
>>> we are
>> struggling to fix flaky tests as it is.  We could delete them, but
>>> those
>> tests were written for a reason (I hope).  Or we could leave them.
>> This
>> pollutes searches etc as inactive code requiring upkeep at least.
>> 
>> I don’t have an easy answer. Some have suggested deleting them. I
>> tend
>>> to
>> lean that direction, but I thought I would consult the community for
>> a
>> broader perspective.
>> 
>> Thanks,
>> Mark
>>> 
>>> 
>> 

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

[Owen Nichols]

It sounds like there is value in being able to deliver un-squashed PRs, and
we believe the richer commit message history outweighs any potential
inconvenience to bisect operations (as Aaron pointed out, finer-grained
commits should generally be to the benefit of bisect operations).

We will leave all 3 merge options enabled in GitHub.

On Tue, Dec 31, 2019 at 7:27 PM Dan Smith  wrote:

> I also tend to do the same workflow Aaron described - make a refactoring
> change to support a feature followed by the actual change I want to make.
> It makes the history a lot clearer because refactoring tends to touch a lot
> of files, so it's nice to have those changes clearly marked as a
> refactoring that should not change behavior.
>
> It's possible to do this as to separate PRs, but that drags out the process
> because you have to get the first in merged before you create the second.
> That encourages bunching changes in a single squashed commit, which makes
> git blame less useful.
>
>
> -Dan
>
> On Tue, Dec 31, 2019, 6:36 PM Nabarun Nag  wrote:
>
> > Aaron ,
> >
> > Is it not the case currently? If I am working on a feature modifying
> class
> > X and another developer makes some refactoring changes on class X and
> > pushes it to develop, won't I have to resolve the merge commits anyway.
> >
> >
> > Regards
> > Naba
> >
> >
> > On Tue, Dec 31, 2019 at 6:01 PM Aaron Lindsey 
> > wrote:
> >
> > > Suppose I’m refactoring the same classes I’m touching for the feature.
> I
> > > do the refactoring on one branch and submit a PR for that. Then I
> > implement
> > > the feature on another branch which is based on develop and does not
> have
> > > the refactoring changes since those are not merged yet. I’ll have to
> > > resolve conflicts on the second PR that I merge.
> > >
> > > Having interdependent PRs where one PR branch is based on another PR
> > > branch is not something I’ve tried. That requires more overhead in
> having
> > > to create more PRs and branches. And if you have N interdependent PRs,
> > you
> > > have to do N-1 merges each time a PR is merged to develop (to update
> the
> > > other branches). It could be done, but is it worth the hassle?
> > >
> > > One more point about rebase-and-merge is that it seems like this would
> > > make bisecting a failure easier since there would be smaller commits.
> > >
> > > Aaron
> > >
> > > > On Dec 31, 2019, at 5:41 PM, Owen Nichols 
> wrote:
> > > >
> > > > Can you elaborate on why you would have to deal with conflicts if the
> > > > refactoring work was kept as a separate PR from the fix?
> > > >
> > > > As with many git workflows, there’s an easy way and a hard way to
> > manage
> > > > interdependent PRs (tl;dr only merge, never rebase!). I wonder if
> this
> > > > points out an opportunity for a “tips and tricks” page on the Geode
> > wiki.
> > > >
> > > > On Tue, Dec 31, 2019 at 5:22 PM Aaron Lindsey <
> aaronlind...@apache.org
> > >
> > > > wrote:
> > > >
> > > >> -0.9
> > > >>
> > > >> I’m not in favor of the revised proposal that disallows
> > > rebase-and-merge.
> > > >> Say I am working on a PR and I have a refactor commit and another
> > commit
> > > >> which implements a new feature. I don’t want those commits to get
> > > squashed
> > > >> because that makes it hard to understand the diff. However, if I
> make
> > > those
> > > >> commits as two separate PRs then I am going to have to deal with
> > > conflicts.
> > > >>
> > > >> I’m not sure when we made it a rule that every commit in develop had
> > to
> > > >> compile and pass tests. I know we implemented a rule that all PRs
> had
> > to
> > > >> pass certain checks, but I never thought that rule implied all
> commits
> > > had
> > > >> to pass those checks.
> > > >>
> > > >> In general I just don’t see the problem with rebase-and-merge and
> this
> > > >> feels like an unnecessary restriction, but I will go with it if
> that’s
> > > what
> > > >> everyone wants to do.
> > > >>
> > > >> Aaron
> > > >>
> > > >>> On Dec 31, 2019, at 3:09 PM, Owen Nichols 
> > wrote:
> > > >>>
> > > >>> To recap, this proposal is now revised to remove 2 of the 3 merge
> > > options
> > > >>> from GitHub, leaving only Squash and Merge.  PR #4513
> > > >>>  serves as an exhibit
> of
> > > >> what is
> > > >>> proposed (it is not to be merged unless discussion leads to
> consensus
> > > >> and a
> > > >>> successful vote).  Please use the dev list (not the PR) for all
> > > >> discussion
> > > >>> (and voting, if we get that far).
> > > >>>
> > > >>> Squash and merge is already used almost exclusively by the Geode
> > > >> community,
> > > >>> with any exceptions tending to be accidental more often than
> > > intentional.
> > > >>> However, some have raised the concern that implementing this
> > > restriction
> > > >>> could result in harm or wasted time.  Can someone give an example
> of
> > a
> > > >> such
> > > >>> a scenario?
> > > >>>
> > > >>> It seems there is a divide here