You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Nabarun Nag <nn...@apache.org> on 2019/10/24 21:24:49 UTC
Fwd: [PSA] Github branch protection
Hi, Geode dev Community,
This is an announcement that the GitHub branch protection rules are *now
active* on develop branch for Apache Geode.
The following rules are currently active :
- Require pull request reviews before merging - at least 1
- Require status checks to pass before merging
[Only for
- concourse-ci/Build
- concourse-ci/UnitTestOpenJDK11
- concourse-ci/UnitTestOpenJDK8
- concourse-ci/StressNewTestOpenJDK11]
After we stabilize the remaining test suites, we can add them to these rule
sets.
Also reminding the community to use squash merge while closing pull
requests.
Regards
Naba
Re: [PSA] Github branch protection
Posted by Aaron Lindsey <al...@pivotal.io>.
Thanks, Naba. Are we now blocking merges for PRs that have
changes requested?
I have a PR right now where instead of the merge button it has the message,
"Merging can be performed automatically once the requested changes are
addressed." It also happens to not have any approvals, so it's possible
that the merge is actually blocked because there are no approvals. In this
case I find the message misleading.
- Aaron
On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
> Hi, Geode dev Community,
>
> This is an announcement that the GitHub branch protection rules are *now
> active* on develop branch for Apache Geode.
>
> The following rules are currently active :
> - Require pull request reviews before merging - at least 1
> - Require status checks to pass before merging
> [Only for
> - concourse-ci/Build
> - concourse-ci/UnitTestOpenJDK11
> - concourse-ci/UnitTestOpenJDK8
> - concourse-ci/StressNewTestOpenJDK11]
>
> After we stabilize the remaining test suites, we can add them to these rule
> sets.
>
> Also reminding the community to use squash merge while closing pull
> requests.
>
> Regards
> Naba
>
[DISCUSS] review process documentation
Posted by Owen Nichols <on...@pivotal.io>.
Below is everything I could find addressing our current process around making and reviewing commits. If you’re a newer contributor, what topics are missing or unclear? If you’ve been a committer for a long time, does what’s in the wiki align with your understanding? Are there unwritten norms that might not be obvious to a new contributor? Thanks for taking a few minutes to review.
VOTEs:
June 2017: Require committers to use GitHub PR’s <https://lists.apache.org/thread.html/cc75b4743a3ecc4a7e278ba285d0d883822a7bc6079ad748f6a50d1e@%3Cdev.geode.apache.org%3E>
DISCUSSions:
Oct 2019: Blocking merge button in PR <https://lists.apache.org/thread.html/c3eb5c028cb3a4d76024f928a7a33c0311228f5dbbcaa86287bf5826@%3Cdev.geode.apache.org%3E>
Oct 2019: No-one is following our commit message formatting <https://lists.apache.org/thread.html/e2e42ab3b9687efb2baecd27bc17fe033437a9ee3f0b79636cb663f0@%3Cdev.geode.apache.org%3E>
Jun 2019: Adopting a coding standard <https://lists.apache.org/thread.html/5dc454470132cc59ed5e82db27fb359fd309adc7997cafc16f3b2dc7@%3Cdev.geode.apache.org%3E>
Sep 2018: Commit message formatting <https://lists.apache.org/thread.html/016ee4921908f2d1bdad4a28c15b2af93f4908aa898290db6f7c17b5@%3Cdev.geode.apache.org%3E>
Sep 2018: Test code style (particularly logging) <https://lists.apache.org/thread.html/86c94c9d3c734f29ecb7440de567f4b25832279f027e78c7f57ebd3e@%3Cdev.geode.apache.org%3E>
Sep 2018: Should we evaluate commit messages as part of PR review? <https://lists.apache.org/thread.html/016ee4921908f2d1bdad4a28c15b2af93f4908aa898290db6f7c17b5@%3Cdev.geode.apache.org%3E>
Feb 2017: JIRA guidelines <https://lists.apache.org/thread.html/bdd74ea6ad7f39ed27df1d3679340b5d2197aeff8f8c9c5b08c211d4@%3Cdev.geode.apache.org%3E>
WIKI pages:
Aug 2018: Criteria for Code Submissions <https://cwiki.apache.org/confluence/display/GEODE/Criteria+for+Code+Submissions?src=contextnavpagetreemode>
Sep 2017: Code contributions <https://cwiki.apache.org/confluence/display/GEODE/Code+contributions?src=contextnavpagetreemode>
Feb 2017: JIRA Guidelines <https://cwiki.apache.org/confluence/display/GEODE/JIRA+Guidelines?src=contextnavpagetreemode>
PR template:
Mar 2019: PULL_REQUEST_TEMPLATE.md <https://github.com/apache/geode/blob/develop/.github/PULL_REQUEST_TEMPLATE.md>
Re: [PSA] Github branch protection
Posted by Owen Nichols <on...@pivotal.io>.
I’d love to know what is our *current* process before discussing changing it. Is there a link, or are new folks expected to read through every email from the past 5 years to establish context?
> On Oct 25, 2019, at 11:49 AM, Kirk Lund <kl...@apache.org> wrote:
>
> We've been operating under a different process than what you are quoting --
> I'm not saying that our current process is correct (or even wrong), however
> it has been different than the quoted process in some ways.
>
> We previously reached consensus for our own process during the first couple
> years of this community simply by doing what we are now doing (right or
> wrong). I assume that changing it requires some sort of formalized
> discussion on this dev-list. Please don't assume or state that we're
> following the quoted process (even if that's what the Apache websites say
> we should be doing). Instead, please propose following the quoted Apache
> process with your interpretation, so that we can establish that as the new
> process by consensus.
>
> On Fri, Oct 25, 2019 at 11:21 AM Owen Nichols <on...@pivotal.io> wrote:
>
>> Successful Apache projects value a broad and collaborative community over
>> the details of the code itself. I don’t think the goal is to hammer out
>> rules like “you can’t merge if someone has requested changes” or “anyone
>> has the right to overrule someone’s request for changes”.
>>
>> Think of “request for changes” as an opportunity to collaborate. From
>> https://www.freecodecamp.org/news/code-review-the-ultimate-guide-aa45c358bbf5/
>> <
>> https://www.freecodecamp.org/news/code-review-the-ultimate-guide-aa45c358bbf5/>
>> :
>>
>> “Code reviews are discussions, not dictation — You can think of most code
>> review feedback as a suggestion more than an order. It’s fine to disagree
>> with a reviewer’s feedback but you need to explain why and give them an
>> opportunity to respond."
>>
>> For the “rules" that we have discussed and agreed on, it would be really
>> helpful if someone could collect them on a single wiki page somewhere,
>> otherwise newcomers to the project face a daunting task of combing through
>> years of email archives to reconstruct all of what has been discussed and
>> decided by the community.
>>
>> -Owen
>>
>>> On Oct 25, 2019, at 9:52 AM, Aaron Lindsey <al...@pivotal.io> wrote:
>>>
>>> @Owen I'm fine with following the "requesting changes is the same as -1"
>>> rule, but I don't think there is consensus from the whole community on
>> this
>>> yet. I was previously told that contributors should make every effort to
>>> address the requested changes, but unless a committer actually comments
>>> "-1" on the PR, the author retains the ability to reject the requested
>>> changes. This probably deserves a separate discussion at some point.
>>>
>>> My original concern has been addressed since Helena pointed out that a
>>> review can be dismissed. I agree this power should probably only be used
>> if
>>> the reviewer cannot be contacted.
>>>
>>> - Aaron
>>>
>>>
>>> On Thu, Oct 24, 2019 at 5:04 PM Owen Nichols <onichols@pivotal.io
>> <ma...@pivotal.io>> wrote:
>>>
>>>>>
>>>>> @Owen It's unclear to me whether "requesting changes" is the same thing
>>>> as
>>>>> a -1 vote. I had previously discussed this with some other committers
>> who
>>>>> were under the impression that they were not the same thing.
>>>>>
>>>>
>>>> If that’s not a -1, what is?
>>>>
>>>> Many apache projects started out long ago when patches were submitted by
>>>> email and voted over email. We have adopted modern GitHub tools to
>>>> streamline this process, but the concept is still the same: reviewers
>> can
>>>> give a +1 (green check) review, or a -1 (with explanation of what it
>> would
>>>> take to get their approval).
>>>>
>>>> One time when I was a relatively new committer, I merged a PR while
>>>> someone still had changes requested on it, and I was admonished: “it
>> sets a
>>>> bad precedent for merges to occur on PRs that have unresolved reviews. <
>>>> https://github.com/apache/geode/pull/3597#issuecomment-493624748 <
>> https://github.com/apache/geode/pull/3597#issuecomment-493624748>>”.
>>>> GitHub will also permanently record that the PR was merged in spite of
>> an
>>>> outstanding request for changes, leading to life-long shame.
>>>>
>>>> If someone has requested changes on your PR, you should make every
>> effort
>>>> to understand and address their concern. If you have pushed additional
>>>> changes since their review, remind them by clicking the re-request
>> review
>>>> button next to their name.
>>>>
>>>> On the flip side, if you request changes on a PR, please clearly explain
>>>> what it would take to gain your approval, and please make an effort to
>>>> re-review in a timely fashion after changes are made. More guideline
>> for
>>>> good PR review practices can be found here <
>>>>
>> https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c
>> <
>> https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c
>>>
>>>>> .
>>>>
>>>> The option to dismiss a review should be a last resort, for example if
>> the
>>>> reviewer went on vacation and cannot be contacted.
>>>>
>>>> -Owen
>>>>
>>>>> - Aaron
>>>>>
>>>>>
>>>>> On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
>>>>>
>>>>>> @Aaron : which PR are you referring to? I can only see "GEODE-7326:
>> Add
>>>>>> cache gets timers" which can be merged? I can get some more idea when
>> I
>>>> can
>>>>>> see whats going on.
>>>>>>
>>>>>> Regards
>>>>>> Naba
>>>>>>
>>>>>> @Kirk : let me run some experiments.
>>>>>>
>>>>>> Regards
>>>>>> Naba
>>>>>>
>>>>>>
>>>>>> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io>
>> wrote:
>>>>>>
>>>>>>> To Kirk's point, there is actually a way to dismiss requests for
>>>> review.
>>>>>>> Info here:
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
>>>>>>> There's instructions in there for how to dismiss a request for
>> changes.
>>>>>> Not
>>>>>>> everyone can do that, so if you aren't a contributor yet you'll
>>>> probably
>>>>>>> have to hit up a current contributor to get any requests for changes
>>>>>>> dismissed.
>>>>>>>
>>>>>>> On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
>>>>>>>
>>>>>>>> One side effect is that any single request for changes will now
>>>>>>> completely
>>>>>>>> block merging the PR. I'm not certain this was intentional? One
>> rogue
>>>>>>>> developer could block the merging of any or every PR. I'm not sure
>> one
>>>>>>>> person should have that much power...
>>>>>>>>
>>>>>>>> On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org>
>> wrote:
>>>>>>>>
>>>>>>>>> Hi, Geode dev Community,
>>>>>>>>>
>>>>>>>>> This is an announcement that the GitHub branch protection rules are
>>>>>>> *now
>>>>>>>>> active* on develop branch for Apache Geode.
>>>>>>>>>
>>>>>>>>> The following rules are currently active :
>>>>>>>>> - Require pull request reviews before merging - at least 1
>>>>>>>>> - Require status checks to pass before merging
>>>>>>>>> [Only for
>>>>>>>>> - concourse-ci/Build
>>>>>>>>> - concourse-ci/UnitTestOpenJDK11
>>>>>>>>> - concourse-ci/UnitTestOpenJDK8
>>>>>>>>> - concourse-ci/StressNewTestOpenJDK11]
>>>>>>>>>
>>>>>>>>> After we stabilize the remaining test suites, we can add them to
>>>>>> these
>>>>>>>> rule
>>>>>>>>> sets.
>>>>>>>>>
>>>>>>>>> Also reminding the community to use squash merge while closing pull
>>>>>>>>> requests.
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> Naba
>>
>>
Re: [PSA] Github branch protection
Posted by Kirk Lund <kl...@apache.org>.
We've been operating under a different process than what you are quoting --
I'm not saying that our current process is correct (or even wrong), however
it has been different than the quoted process in some ways.
We previously reached consensus for our own process during the first couple
years of this community simply by doing what we are now doing (right or
wrong). I assume that changing it requires some sort of formalized
discussion on this dev-list. Please don't assume or state that we're
following the quoted process (even if that's what the Apache websites say
we should be doing). Instead, please propose following the quoted Apache
process with your interpretation, so that we can establish that as the new
process by consensus.
On Fri, Oct 25, 2019 at 11:21 AM Owen Nichols <on...@pivotal.io> wrote:
> Successful Apache projects value a broad and collaborative community over
> the details of the code itself. I don’t think the goal is to hammer out
> rules like “you can’t merge if someone has requested changes” or “anyone
> has the right to overrule someone’s request for changes”.
>
> Think of “request for changes” as an opportunity to collaborate. From
> https://www.freecodecamp.org/news/code-review-the-ultimate-guide-aa45c358bbf5/
> <
> https://www.freecodecamp.org/news/code-review-the-ultimate-guide-aa45c358bbf5/>
> :
>
> “Code reviews are discussions, not dictation — You can think of most code
> review feedback as a suggestion more than an order. It’s fine to disagree
> with a reviewer’s feedback but you need to explain why and give them an
> opportunity to respond."
>
> For the “rules" that we have discussed and agreed on, it would be really
> helpful if someone could collect them on a single wiki page somewhere,
> otherwise newcomers to the project face a daunting task of combing through
> years of email archives to reconstruct all of what has been discussed and
> decided by the community.
>
> -Owen
>
> > On Oct 25, 2019, at 9:52 AM, Aaron Lindsey <al...@pivotal.io> wrote:
> >
> > @Owen I'm fine with following the "requesting changes is the same as -1"
> > rule, but I don't think there is consensus from the whole community on
> this
> > yet. I was previously told that contributors should make every effort to
> > address the requested changes, but unless a committer actually comments
> > "-1" on the PR, the author retains the ability to reject the requested
> > changes. This probably deserves a separate discussion at some point.
> >
> > My original concern has been addressed since Helena pointed out that a
> > review can be dismissed. I agree this power should probably only be used
> if
> > the reviewer cannot be contacted.
> >
> > - Aaron
> >
> >
> > On Thu, Oct 24, 2019 at 5:04 PM Owen Nichols <onichols@pivotal.io
> <ma...@pivotal.io>> wrote:
> >
> >>>
> >>> @Owen It's unclear to me whether "requesting changes" is the same thing
> >> as
> >>> a -1 vote. I had previously discussed this with some other committers
> who
> >>> were under the impression that they were not the same thing.
> >>>
> >>
> >> If that’s not a -1, what is?
> >>
> >> Many apache projects started out long ago when patches were submitted by
> >> email and voted over email. We have adopted modern GitHub tools to
> >> streamline this process, but the concept is still the same: reviewers
> can
> >> give a +1 (green check) review, or a -1 (with explanation of what it
> would
> >> take to get their approval).
> >>
> >> One time when I was a relatively new committer, I merged a PR while
> >> someone still had changes requested on it, and I was admonished: “it
> sets a
> >> bad precedent for merges to occur on PRs that have unresolved reviews. <
> >> https://github.com/apache/geode/pull/3597#issuecomment-493624748 <
> https://github.com/apache/geode/pull/3597#issuecomment-493624748>>”.
> >> GitHub will also permanently record that the PR was merged in spite of
> an
> >> outstanding request for changes, leading to life-long shame.
> >>
> >> If someone has requested changes on your PR, you should make every
> effort
> >> to understand and address their concern. If you have pushed additional
> >> changes since their review, remind them by clicking the re-request
> review
> >> button next to their name.
> >>
> >> On the flip side, if you request changes on a PR, please clearly explain
> >> what it would take to gain your approval, and please make an effort to
> >> re-review in a timely fashion after changes are made. More guideline
> for
> >> good PR review practices can be found here <
> >>
> https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c
> <
> https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c
> >
> >>> .
> >>
> >> The option to dismiss a review should be a last resort, for example if
> the
> >> reviewer went on vacation and cannot be contacted.
> >>
> >> -Owen
> >>
> >>> - Aaron
> >>>
> >>>
> >>> On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
> >>>
> >>>> @Aaron : which PR are you referring to? I can only see "GEODE-7326:
> Add
> >>>> cache gets timers" which can be merged? I can get some more idea when
> I
> >> can
> >>>> see whats going on.
> >>>>
> >>>> Regards
> >>>> Naba
> >>>>
> >>>> @Kirk : let me run some experiments.
> >>>>
> >>>> Regards
> >>>> Naba
> >>>>
> >>>>
> >>>> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io>
> wrote:
> >>>>
> >>>>> To Kirk's point, there is actually a way to dismiss requests for
> >> review.
> >>>>> Info here:
> >>>>>
> >>>>>
> >>>>
> >>
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
> >>>>> There's instructions in there for how to dismiss a request for
> changes.
> >>>> Not
> >>>>> everyone can do that, so if you aren't a contributor yet you'll
> >> probably
> >>>>> have to hit up a current contributor to get any requests for changes
> >>>>> dismissed.
> >>>>>
> >>>>> On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
> >>>>>
> >>>>>> One side effect is that any single request for changes will now
> >>>>> completely
> >>>>>> block merging the PR. I'm not certain this was intentional? One
> rogue
> >>>>>> developer could block the merging of any or every PR. I'm not sure
> one
> >>>>>> person should have that much power...
> >>>>>>
> >>>>>> On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org>
> wrote:
> >>>>>>
> >>>>>>> Hi, Geode dev Community,
> >>>>>>>
> >>>>>>> This is an announcement that the GitHub branch protection rules are
> >>>>> *now
> >>>>>>> active* on develop branch for Apache Geode.
> >>>>>>>
> >>>>>>> The following rules are currently active :
> >>>>>>> - Require pull request reviews before merging - at least 1
> >>>>>>> - Require status checks to pass before merging
> >>>>>>> [Only for
> >>>>>>> - concourse-ci/Build
> >>>>>>> - concourse-ci/UnitTestOpenJDK11
> >>>>>>> - concourse-ci/UnitTestOpenJDK8
> >>>>>>> - concourse-ci/StressNewTestOpenJDK11]
> >>>>>>>
> >>>>>>> After we stabilize the remaining test suites, we can add them to
> >>>> these
> >>>>>> rule
> >>>>>>> sets.
> >>>>>>>
> >>>>>>> Also reminding the community to use squash merge while closing pull
> >>>>>>> requests.
> >>>>>>>
> >>>>>>> Regards
> >>>>>>> Naba
>
>
Re: [PSA] Github branch protection
Posted by Owen Nichols <on...@pivotal.io>.
Successful Apache projects value a broad and collaborative community over the details of the code itself. I don’t think the goal is to hammer out rules like “you can’t merge if someone has requested changes” or “anyone has the right to overrule someone’s request for changes”.
Think of “request for changes” as an opportunity to collaborate. From https://www.freecodecamp.org/news/code-review-the-ultimate-guide-aa45c358bbf5/ <https://www.freecodecamp.org/news/code-review-the-ultimate-guide-aa45c358bbf5/> :
“Code reviews are discussions, not dictation — You can think of most code review feedback as a suggestion more than an order. It’s fine to disagree with a reviewer’s feedback but you need to explain why and give them an opportunity to respond."
For the “rules" that we have discussed and agreed on, it would be really helpful if someone could collect them on a single wiki page somewhere, otherwise newcomers to the project face a daunting task of combing through years of email archives to reconstruct all of what has been discussed and decided by the community.
-Owen
> On Oct 25, 2019, at 9:52 AM, Aaron Lindsey <al...@pivotal.io> wrote:
>
> @Owen I'm fine with following the "requesting changes is the same as -1"
> rule, but I don't think there is consensus from the whole community on this
> yet. I was previously told that contributors should make every effort to
> address the requested changes, but unless a committer actually comments
> "-1" on the PR, the author retains the ability to reject the requested
> changes. This probably deserves a separate discussion at some point.
>
> My original concern has been addressed since Helena pointed out that a
> review can be dismissed. I agree this power should probably only be used if
> the reviewer cannot be contacted.
>
> - Aaron
>
>
> On Thu, Oct 24, 2019 at 5:04 PM Owen Nichols <onichols@pivotal.io <ma...@pivotal.io>> wrote:
>
>>>
>>> @Owen It's unclear to me whether "requesting changes" is the same thing
>> as
>>> a -1 vote. I had previously discussed this with some other committers who
>>> were under the impression that they were not the same thing.
>>>
>>
>> If that’s not a -1, what is?
>>
>> Many apache projects started out long ago when patches were submitted by
>> email and voted over email. We have adopted modern GitHub tools to
>> streamline this process, but the concept is still the same: reviewers can
>> give a +1 (green check) review, or a -1 (with explanation of what it would
>> take to get their approval).
>>
>> One time when I was a relatively new committer, I merged a PR while
>> someone still had changes requested on it, and I was admonished: “it sets a
>> bad precedent for merges to occur on PRs that have unresolved reviews. <
>> https://github.com/apache/geode/pull/3597#issuecomment-493624748 <https://github.com/apache/geode/pull/3597#issuecomment-493624748>>”.
>> GitHub will also permanently record that the PR was merged in spite of an
>> outstanding request for changes, leading to life-long shame.
>>
>> If someone has requested changes on your PR, you should make every effort
>> to understand and address their concern. If you have pushed additional
>> changes since their review, remind them by clicking the re-request review
>> button next to their name.
>>
>> On the flip side, if you request changes on a PR, please clearly explain
>> what it would take to gain your approval, and please make an effort to
>> re-review in a timely fashion after changes are made. More guideline for
>> good PR review practices can be found here <
>> https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c <https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c>
>>> .
>>
>> The option to dismiss a review should be a last resort, for example if the
>> reviewer went on vacation and cannot be contacted.
>>
>> -Owen
>>
>>> - Aaron
>>>
>>>
>>> On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
>>>
>>>> @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
>>>> cache gets timers" which can be merged? I can get some more idea when I
>> can
>>>> see whats going on.
>>>>
>>>> Regards
>>>> Naba
>>>>
>>>> @Kirk : let me run some experiments.
>>>>
>>>> Regards
>>>> Naba
>>>>
>>>>
>>>> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io> wrote:
>>>>
>>>>> To Kirk's point, there is actually a way to dismiss requests for
>> review.
>>>>> Info here:
>>>>>
>>>>>
>>>>
>> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
>>>>> There's instructions in there for how to dismiss a request for changes.
>>>> Not
>>>>> everyone can do that, so if you aren't a contributor yet you'll
>> probably
>>>>> have to hit up a current contributor to get any requests for changes
>>>>> dismissed.
>>>>>
>>>>> On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
>>>>>
>>>>>> One side effect is that any single request for changes will now
>>>>> completely
>>>>>> block merging the PR. I'm not certain this was intentional? One rogue
>>>>>> developer could block the merging of any or every PR. I'm not sure one
>>>>>> person should have that much power...
>>>>>>
>>>>>> On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
>>>>>>
>>>>>>> Hi, Geode dev Community,
>>>>>>>
>>>>>>> This is an announcement that the GitHub branch protection rules are
>>>>> *now
>>>>>>> active* on develop branch for Apache Geode.
>>>>>>>
>>>>>>> The following rules are currently active :
>>>>>>> - Require pull request reviews before merging - at least 1
>>>>>>> - Require status checks to pass before merging
>>>>>>> [Only for
>>>>>>> - concourse-ci/Build
>>>>>>> - concourse-ci/UnitTestOpenJDK11
>>>>>>> - concourse-ci/UnitTestOpenJDK8
>>>>>>> - concourse-ci/StressNewTestOpenJDK11]
>>>>>>>
>>>>>>> After we stabilize the remaining test suites, we can add them to
>>>> these
>>>>>> rule
>>>>>>> sets.
>>>>>>>
>>>>>>> Also reminding the community to use squash merge while closing pull
>>>>>>> requests.
>>>>>>>
>>>>>>> Regards
>>>>>>> Naba
Re: [PSA] Github branch protection
Posted by Aaron Lindsey <al...@pivotal.io>.
@Owen I'm fine with following the "requesting changes is the same as -1"
rule, but I don't think there is consensus from the whole community on this
yet. I was previously told that contributors should make every effort to
address the requested changes, but unless a committer actually comments
"-1" on the PR, the author retains the ability to reject the requested
changes. This probably deserves a separate discussion at some point.
My original concern has been addressed since Helena pointed out that a
review can be dismissed. I agree this power should probably only be used if
the reviewer cannot be contacted.
- Aaron
On Thu, Oct 24, 2019 at 5:04 PM Owen Nichols <on...@pivotal.io> wrote:
> >
> > @Owen It's unclear to me whether "requesting changes" is the same thing
> as
> > a -1 vote. I had previously discussed this with some other committers who
> > were under the impression that they were not the same thing.
> >
>
> If that’s not a -1, what is?
>
> Many apache projects started out long ago when patches were submitted by
> email and voted over email. We have adopted modern GitHub tools to
> streamline this process, but the concept is still the same: reviewers can
> give a +1 (green check) review, or a -1 (with explanation of what it would
> take to get their approval).
>
> One time when I was a relatively new committer, I merged a PR while
> someone still had changes requested on it, and I was admonished: “it sets a
> bad precedent for merges to occur on PRs that have unresolved reviews. <
> https://github.com/apache/geode/pull/3597#issuecomment-493624748>”.
> GitHub will also permanently record that the PR was merged in spite of an
> outstanding request for changes, leading to life-long shame.
>
> If someone has requested changes on your PR, you should make every effort
> to understand and address their concern. If you have pushed additional
> changes since their review, remind them by clicking the re-request review
> button next to their name.
>
> On the flip side, if you request changes on a PR, please clearly explain
> what it would take to gain your approval, and please make an effort to
> re-review in a timely fashion after changes are made. More guideline for
> good PR review practices can be found here <
> https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c
> >.
>
> The option to dismiss a review should be a last resort, for example if the
> reviewer went on vacation and cannot be contacted.
>
> -Owen
>
> > - Aaron
> >
> >
> > On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
> >
> >> @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
> >> cache gets timers" which can be merged? I can get some more idea when I
> can
> >> see whats going on.
> >>
> >> Regards
> >> Naba
> >>
> >> @Kirk : let me run some experiments.
> >>
> >> Regards
> >> Naba
> >>
> >>
> >> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io> wrote:
> >>
> >>> To Kirk's point, there is actually a way to dismiss requests for
> review.
> >>> Info here:
> >>>
> >>>
> >>
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
> >>> There's instructions in there for how to dismiss a request for changes.
> >> Not
> >>> everyone can do that, so if you aren't a contributor yet you'll
> probably
> >>> have to hit up a current contributor to get any requests for changes
> >>> dismissed.
> >>>
> >>> On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
> >>>
> >>>> One side effect is that any single request for changes will now
> >>> completely
> >>>> block merging the PR. I'm not certain this was intentional? One rogue
> >>>> developer could block the merging of any or every PR. I'm not sure one
> >>>> person should have that much power...
> >>>>
> >>>> On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
> >>>>
> >>>>> Hi, Geode dev Community,
> >>>>>
> >>>>> This is an announcement that the GitHub branch protection rules are
> >>> *now
> >>>>> active* on develop branch for Apache Geode.
> >>>>>
> >>>>> The following rules are currently active :
> >>>>> - Require pull request reviews before merging - at least 1
> >>>>> - Require status checks to pass before merging
> >>>>> [Only for
> >>>>> - concourse-ci/Build
> >>>>> - concourse-ci/UnitTestOpenJDK11
> >>>>> - concourse-ci/UnitTestOpenJDK8
> >>>>> - concourse-ci/StressNewTestOpenJDK11]
> >>>>>
> >>>>> After we stabilize the remaining test suites, we can add them to
> >> these
> >>>> rule
> >>>>> sets.
> >>>>>
> >>>>> Also reminding the community to use squash merge while closing pull
> >>>>> requests.
> >>>>>
> >>>>> Regards
> >>>>> Naba
> >>>>>
> >>>>
> >>>
> >>
>
>
Re: [PSA] Github branch protection
Posted by Owen Nichols <on...@pivotal.io>.
>
> @Owen It's unclear to me whether "requesting changes" is the same thing as
> a -1 vote. I had previously discussed this with some other committers who
> were under the impression that they were not the same thing.
>
If that’s not a -1, what is?
Many apache projects started out long ago when patches were submitted by email and voted over email. We have adopted modern GitHub tools to streamline this process, but the concept is still the same: reviewers can give a +1 (green check) review, or a -1 (with explanation of what it would take to get their approval).
One time when I was a relatively new committer, I merged a PR while someone still had changes requested on it, and I was admonished: “it sets a bad precedent for merges to occur on PRs that have unresolved reviews. <https://github.com/apache/geode/pull/3597#issuecomment-493624748>”. GitHub will also permanently record that the PR was merged in spite of an outstanding request for changes, leading to life-long shame.
If someone has requested changes on your PR, you should make every effort to understand and address their concern. If you have pushed additional changes since their review, remind them by clicking the re-request review button next to their name.
On the flip side, if you request changes on a PR, please clearly explain what it would take to gain your approval, and please make an effort to re-review in a timely fashion after changes are made. More guideline for good PR review practices can be found here <https://medium.freecodecamp.org/unlearning-toxic-behaviors-in-a-code-review-culture-b7c295452a3c>.
The option to dismiss a review should be a last resort, for example if the reviewer went on vacation and cannot be contacted.
-Owen
> - Aaron
>
>
> On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
>
>> @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
>> cache gets timers" which can be merged? I can get some more idea when I can
>> see whats going on.
>>
>> Regards
>> Naba
>>
>> @Kirk : let me run some experiments.
>>
>> Regards
>> Naba
>>
>>
>> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io> wrote:
>>
>>> To Kirk's point, there is actually a way to dismiss requests for review.
>>> Info here:
>>>
>>>
>> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
>>> There's instructions in there for how to dismiss a request for changes.
>> Not
>>> everyone can do that, so if you aren't a contributor yet you'll probably
>>> have to hit up a current contributor to get any requests for changes
>>> dismissed.
>>>
>>> On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
>>>
>>>> One side effect is that any single request for changes will now
>>> completely
>>>> block merging the PR. I'm not certain this was intentional? One rogue
>>>> developer could block the merging of any or every PR. I'm not sure one
>>>> person should have that much power...
>>>>
>>>> On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
>>>>
>>>>> Hi, Geode dev Community,
>>>>>
>>>>> This is an announcement that the GitHub branch protection rules are
>>> *now
>>>>> active* on develop branch for Apache Geode.
>>>>>
>>>>> The following rules are currently active :
>>>>> - Require pull request reviews before merging - at least 1
>>>>> - Require status checks to pass before merging
>>>>> [Only for
>>>>> - concourse-ci/Build
>>>>> - concourse-ci/UnitTestOpenJDK11
>>>>> - concourse-ci/UnitTestOpenJDK8
>>>>> - concourse-ci/StressNewTestOpenJDK11]
>>>>>
>>>>> After we stabilize the remaining test suites, we can add them to
>> these
>>>> rule
>>>>> sets.
>>>>>
>>>>> Also reminding the community to use squash merge while closing pull
>>>>> requests.
>>>>>
>>>>> Regards
>>>>> Naba
>>>>>
>>>>
>>>
>>
Re: [PSA] Github branch protection
Posted by Nabarun Nag <nn...@apache.org>.
Edit: Helena has mentioned that a review can be dismissed.
[spelling mistake]
On Thu, Oct 24, 2019 at 4:35 PM Nabarun Nag <nn...@apache.org> wrote:
> Hi Aaron / Kirk,
>
> Thank you for the update. As Owen has mentioned that if there is a change
> request from someone with write permission to the branch, we should address
> it, or request a re-review.
> In the "rogue developer" case which Kirk mentioned, and I hope that this
> does not happen in the Apache Geode community, -- Halena has mentioned that
> a review can be dismissed.
>
> Please do let me know if there are any more information requests.
>
> Regards
> Naba
>
>
> On Thu, Oct 24, 2019 at 3:43 PM Aaron Lindsey <al...@pivotal.io> wrote:
>
>> @Naba That's the one. It was approved shortly after I sent that message
>> though. It should be reproducible by requesting changes on a PR with no
>> other reviews.
>>
>> @Owen It's unclear to me whether "requesting changes" is the same thing as
>> a -1 vote. I had previously discussed this with some other committers who
>> were under the impression that they were not the same thing.
>>
>> @Helena Thanks! I didn't know that was possible.
>>
>> - Aaron
>>
>>
>> On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
>>
>> > @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
>> > cache gets timers" which can be merged? I can get some more idea when I
>> can
>> > see whats going on.
>> >
>> > Regards
>> > Naba
>> >
>> > @Kirk : let me run some experiments.
>> >
>> > Regards
>> > Naba
>> >
>> >
>> > On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io> wrote:
>> >
>> > > To Kirk's point, there is actually a way to dismiss requests for
>> review.
>> > > Info here:
>> > >
>> > >
>> >
>> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
>> > > There's instructions in there for how to dismiss a request for
>> changes.
>> > Not
>> > > everyone can do that, so if you aren't a contributor yet you'll
>> probably
>> > > have to hit up a current contributor to get any requests for changes
>> > > dismissed.
>> > >
>> > > On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
>> > >
>> > > > One side effect is that any single request for changes will now
>> > > completely
>> > > > block merging the PR. I'm not certain this was intentional? One
>> rogue
>> > > > developer could block the merging of any or every PR. I'm not sure
>> one
>> > > > person should have that much power...
>> > > >
>> > > > On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org>
>> wrote:
>> > > >
>> > > > > Hi, Geode dev Community,
>> > > > >
>> > > > > This is an announcement that the GitHub branch protection rules
>> are
>> > > *now
>> > > > > active* on develop branch for Apache Geode.
>> > > > >
>> > > > > The following rules are currently active :
>> > > > > - Require pull request reviews before merging - at least 1
>> > > > > - Require status checks to pass before merging
>> > > > > [Only for
>> > > > > - concourse-ci/Build
>> > > > > - concourse-ci/UnitTestOpenJDK11
>> > > > > - concourse-ci/UnitTestOpenJDK8
>> > > > > - concourse-ci/StressNewTestOpenJDK11]
>> > > > >
>> > > > > After we stabilize the remaining test suites, we can add them to
>> > these
>> > > > rule
>> > > > > sets.
>> > > > >
>> > > > > Also reminding the community to use squash merge while closing
>> pull
>> > > > > requests.
>> > > > >
>> > > > > Regards
>> > > > > Naba
>> > > > >
>> > > >
>> > >
>> >
>>
>
Re: [PSA] Github branch protection
Posted by Nabarun Nag <nn...@apache.org>.
Hi Aaron / Kirk,
Thank you for the update. As Owen has mentioned that if there is a change
request from someone with write permission to the branch, we should address
it, or request a re-review.
In the "rogue developer" case which Kirk mentioned, and I hope that this
does not happen in the Apache Geode community, -- Halena has mentioned that
a review can be dismissed.
Please do let me know if there are any more information requests.
Regards
Naba
On Thu, Oct 24, 2019 at 3:43 PM Aaron Lindsey <al...@pivotal.io> wrote:
> @Naba That's the one. It was approved shortly after I sent that message
> though. It should be reproducible by requesting changes on a PR with no
> other reviews.
>
> @Owen It's unclear to me whether "requesting changes" is the same thing as
> a -1 vote. I had previously discussed this with some other committers who
> were under the impression that they were not the same thing.
>
> @Helena Thanks! I didn't know that was possible.
>
> - Aaron
>
>
> On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
>
> > @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
> > cache gets timers" which can be merged? I can get some more idea when I
> can
> > see whats going on.
> >
> > Regards
> > Naba
> >
> > @Kirk : let me run some experiments.
> >
> > Regards
> > Naba
> >
> >
> > On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io> wrote:
> >
> > > To Kirk's point, there is actually a way to dismiss requests for
> review.
> > > Info here:
> > >
> > >
> >
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
> > > There's instructions in there for how to dismiss a request for changes.
> > Not
> > > everyone can do that, so if you aren't a contributor yet you'll
> probably
> > > have to hit up a current contributor to get any requests for changes
> > > dismissed.
> > >
> > > On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
> > >
> > > > One side effect is that any single request for changes will now
> > > completely
> > > > block merging the PR. I'm not certain this was intentional? One rogue
> > > > developer could block the merging of any or every PR. I'm not sure
> one
> > > > person should have that much power...
> > > >
> > > > On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
> > > >
> > > > > Hi, Geode dev Community,
> > > > >
> > > > > This is an announcement that the GitHub branch protection rules are
> > > *now
> > > > > active* on develop branch for Apache Geode.
> > > > >
> > > > > The following rules are currently active :
> > > > > - Require pull request reviews before merging - at least 1
> > > > > - Require status checks to pass before merging
> > > > > [Only for
> > > > > - concourse-ci/Build
> > > > > - concourse-ci/UnitTestOpenJDK11
> > > > > - concourse-ci/UnitTestOpenJDK8
> > > > > - concourse-ci/StressNewTestOpenJDK11]
> > > > >
> > > > > After we stabilize the remaining test suites, we can add them to
> > these
> > > > rule
> > > > > sets.
> > > > >
> > > > > Also reminding the community to use squash merge while closing pull
> > > > > requests.
> > > > >
> > > > > Regards
> > > > > Naba
> > > > >
> > > >
> > >
> >
>
Re: [PSA] Github branch protection
Posted by Aaron Lindsey <al...@pivotal.io>.
@Naba That's the one. It was approved shortly after I sent that message
though. It should be reproducible by requesting changes on a PR with no
other reviews.
@Owen It's unclear to me whether "requesting changes" is the same thing as
a -1 vote. I had previously discussed this with some other committers who
were under the impression that they were not the same thing.
@Helena Thanks! I didn't know that was possible.
- Aaron
On Thu, Oct 24, 2019 at 3:02 PM Nabarun Nag <nn...@apache.org> wrote:
> @Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
> cache gets timers" which can be merged? I can get some more idea when I can
> see whats going on.
>
> Regards
> Naba
>
> @Kirk : let me run some experiments.
>
> Regards
> Naba
>
>
> On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io> wrote:
>
> > To Kirk's point, there is actually a way to dismiss requests for review.
> > Info here:
> >
> >
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
> > There's instructions in there for how to dismiss a request for changes.
> Not
> > everyone can do that, so if you aren't a contributor yet you'll probably
> > have to hit up a current contributor to get any requests for changes
> > dismissed.
> >
> > On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
> >
> > > One side effect is that any single request for changes will now
> > completely
> > > block merging the PR. I'm not certain this was intentional? One rogue
> > > developer could block the merging of any or every PR. I'm not sure one
> > > person should have that much power...
> > >
> > > On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
> > >
> > > > Hi, Geode dev Community,
> > > >
> > > > This is an announcement that the GitHub branch protection rules are
> > *now
> > > > active* on develop branch for Apache Geode.
> > > >
> > > > The following rules are currently active :
> > > > - Require pull request reviews before merging - at least 1
> > > > - Require status checks to pass before merging
> > > > [Only for
> > > > - concourse-ci/Build
> > > > - concourse-ci/UnitTestOpenJDK11
> > > > - concourse-ci/UnitTestOpenJDK8
> > > > - concourse-ci/StressNewTestOpenJDK11]
> > > >
> > > > After we stabilize the remaining test suites, we can add them to
> these
> > > rule
> > > > sets.
> > > >
> > > > Also reminding the community to use squash merge while closing pull
> > > > requests.
> > > >
> > > > Regards
> > > > Naba
> > > >
> > >
> >
>
Re: [PSA] Github branch protection
Posted by Nabarun Nag <nn...@apache.org>.
@Aaron : which PR are you referring to? I can only see "GEODE-7326: Add
cache gets timers" which can be merged? I can get some more idea when I can
see whats going on.
Regards
Naba
@Kirk : let me run some experiments.
Regards
Naba
On Thu, Oct 24, 2019 at 2:57 PM Helena Bales <hb...@pivotal.io> wrote:
> To Kirk's point, there is actually a way to dismiss requests for review.
> Info here:
>
> https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
> There's instructions in there for how to dismiss a request for changes. Not
> everyone can do that, so if you aren't a contributor yet you'll probably
> have to hit up a current contributor to get any requests for changes
> dismissed.
>
> On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
>
> > One side effect is that any single request for changes will now
> completely
> > block merging the PR. I'm not certain this was intentional? One rogue
> > developer could block the merging of any or every PR. I'm not sure one
> > person should have that much power...
> >
> > On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
> >
> > > Hi, Geode dev Community,
> > >
> > > This is an announcement that the GitHub branch protection rules are
> *now
> > > active* on develop branch for Apache Geode.
> > >
> > > The following rules are currently active :
> > > - Require pull request reviews before merging - at least 1
> > > - Require status checks to pass before merging
> > > [Only for
> > > - concourse-ci/Build
> > > - concourse-ci/UnitTestOpenJDK11
> > > - concourse-ci/UnitTestOpenJDK8
> > > - concourse-ci/StressNewTestOpenJDK11]
> > >
> > > After we stabilize the remaining test suites, we can add them to these
> > rule
> > > sets.
> > >
> > > Also reminding the community to use squash merge while closing pull
> > > requests.
> > >
> > > Regards
> > > Naba
> > >
> >
>
Re: [PSA] Github branch protection
Posted by Helena Bales <hb...@pivotal.io>.
To Kirk's point, there is actually a way to dismiss requests for review.
Info here:
https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/dismissing-a-pull-request-review
There's instructions in there for how to dismiss a request for changes. Not
everyone can do that, so if you aren't a contributor yet you'll probably
have to hit up a current contributor to get any requests for changes
dismissed.
On Thu, Oct 24, 2019 at 2:52 PM Kirk Lund <kl...@apache.org> wrote:
> One side effect is that any single request for changes will now completely
> block merging the PR. I'm not certain this was intentional? One rogue
> developer could block the merging of any or every PR. I'm not sure one
> person should have that much power...
>
> On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
>
> > Hi, Geode dev Community,
> >
> > This is an announcement that the GitHub branch protection rules are *now
> > active* on develop branch for Apache Geode.
> >
> > The following rules are currently active :
> > - Require pull request reviews before merging - at least 1
> > - Require status checks to pass before merging
> > [Only for
> > - concourse-ci/Build
> > - concourse-ci/UnitTestOpenJDK11
> > - concourse-ci/UnitTestOpenJDK8
> > - concourse-ci/StressNewTestOpenJDK11]
> >
> > After we stabilize the remaining test suites, we can add them to these
> rule
> > sets.
> >
> > Also reminding the community to use squash merge while closing pull
> > requests.
> >
> > Regards
> > Naba
> >
>
Re: [PSA] Github branch protection
Posted by Owen Nichols <on...@pivotal.io>.
From https://www.apache.org/foundation/voting.html
"A code-modification proposal may be stopped dead in its tracks by a -1 vote by a qualified voter."
So I think this is consistent with Apache guidelines...
> On Oct 24, 2019, at 2:51 PM, Kirk Lund <kl...@apache.org> wrote:
>
> One side effect is that any single request for changes will now completely
> block merging the PR. I'm not certain this was intentional? One rogue
> developer could block the merging of any or every PR. I'm not sure one
> person should have that much power...
>
> On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
>
>> Hi, Geode dev Community,
>>
>> This is an announcement that the GitHub branch protection rules are *now
>> active* on develop branch for Apache Geode.
>>
>> The following rules are currently active :
>> - Require pull request reviews before merging - at least 1
>> - Require status checks to pass before merging
>> [Only for
>> - concourse-ci/Build
>> - concourse-ci/UnitTestOpenJDK11
>> - concourse-ci/UnitTestOpenJDK8
>> - concourse-ci/StressNewTestOpenJDK11]
>>
>> After we stabilize the remaining test suites, we can add them to these rule
>> sets.
>>
>> Also reminding the community to use squash merge while closing pull
>> requests.
>>
>> Regards
>> Naba
>>
Re: [PSA] Github branch protection
Posted by Kirk Lund <kl...@apache.org>.
One side effect is that any single request for changes will now completely
block merging the PR. I'm not certain this was intentional? One rogue
developer could block the merging of any or every PR. I'm not sure one
person should have that much power...
On Thu, Oct 24, 2019 at 2:25 PM Nabarun Nag <nn...@apache.org> wrote:
> Hi, Geode dev Community,
>
> This is an announcement that the GitHub branch protection rules are *now
> active* on develop branch for Apache Geode.
>
> The following rules are currently active :
> - Require pull request reviews before merging - at least 1
> - Require status checks to pass before merging
> [Only for
> - concourse-ci/Build
> - concourse-ci/UnitTestOpenJDK11
> - concourse-ci/UnitTestOpenJDK8
> - concourse-ci/StressNewTestOpenJDK11]
>
> After we stabilize the remaining test suites, we can add them to these rule
> sets.
>
> Also reminding the community to use squash merge while closing pull
> requests.
>
> Regards
> Naba
>