I'm not sure if you can on GitHub, so that would need checking. We're using the 
Microsoft TFS Git system, for historical reasons.

Nick

> On 18 Aug 2017, at 16:55, Joan Touzet <woh...@apache.org> wrote:
> 
> I didn't realize you could review your own PR. That gives us the "escape
> hatch" that we need.
> 
> -Joan
> 
> ----- Original Message -----
> From: "Nick North" <nort...@gmail.com>
> To: dev@couchdb.apache.org, "Joan Touzet" <woh...@apache.org>
> Sent: Friday, 18 August, 2017 9:48:40 AM
> Subject: Re: [DISCUSSION] Disallow all merges of PRs to master that cause 
> tests to fail
> 
> 
> This is pretty much the set of restrictions we have on the master branch in 
> my organisation, and it works well. We also require PR reviews before 
> merging, but anyone in the team can do the review, including the PR author. 
> This means the author has to make a conscious decision on whether the changes 
> are trivial enough to sign off themselves, or whether someone else should 
> review them, and there's an audit trail of that decision being made. 
> 
> 
> Nick 
> 
> 
> 
> On Wed, 16 Aug 2017 at 23:49 Joan Touzet < woh...@apache.org > wrote: 
> 
> 
> Seems there is general consensus. 
> 
> Now, how do people feel about me asking Infra to make this change to the 
> main repos (couchdb, couchdb-fauxton, etc.): 
> 
> https://help.github.com/assets/images/help/repository/protecting-branch-loose-status.png
>  
> 
> Specifically: 
> 
> Protect master branch (and any release branches like 2.1.x) 
> Require status checks to pass before merging 
> Require branches be up to date before merging 
> 
> We can have an optional secondary discussion around enforcing: 
> 
> Require pull request reviews before merging 
> 
> This would enforce our RTC model, but we *need* more active devs if this 
> is going to pass. I've had to beg multiple times for many of my PRs in 
> the 2.1.0 release cycle to be approved...even trivial documentation 
> changes. It was very frustrating. 
> 
> -Joan 
> 
> ----- Original Message ----- 
> From: "Nick Vatamaniuc" < vatam...@gmail.com > 
> To: dev@couchdb.apache.org 
> Sent: Wednesday, 16 August, 2017 6:01:34 PM 
> Subject: Re: [DISCUSSION] Disallow all merges of PRs to master that cause 
> tests to fail 
> 
> +1 
> 
>> On Aug 16, 2017 15:50, "Alexander Shorin" < kxe...@gmail.com > wrote: 
>> 
>> It's strange to say something else than +1 or question the topic in any 
>> way. 
>> 
>> Good call, Joan! 
>> -- 
>> ,,,^..^,,, 
>> 
>> 
>>> On Wed, Aug 16, 2017 at 6:46 AM, Joan Touzet < woh...@apache.org > wrote: 
>>> Hi committers, 
>>> 
>>> I'd like to propose a change to our policy on version control, namely 
>>> that no check-ins be allowed on the master branch unless CI test runs 
>>> against that PR are clean. 
>>> 
>>> We've worked hard as a group to get runs clean. We need to protect 
>>> that achievement and investment in our test suite. That means not 
>>> letting rogue check-ins slip by because we are ignoring a red X in 
>>> GitHub (GH) from the Travis run. 
>>> 
>>> Things I see as exceptions: 
>>> * Changes to things clearly not related to the test suite, i.e. 
>>> documentation, support scripts, rel/overlay/etc/ files, etc. 
>>> * Changes already agreed upon in a previous PR/discussion for 
>>> administrative tasks 
>>> 
>>> Interesting situation right now for a discussion: Garren has a PR up[1] 
>>> that enables the mango tests to be part of the standard Travis/Jenkins 
>>> runs. Unfortunately, it doesn't pass on one of our platforms right now 
>>> and that needs investigation. Should we allow the PR to land and fix 
>>> the problems in master, or should the PR hold-up until it can land along 
>>> with the fixes for the failing mango tests? I can see both sides of this 
>>> argument. 
>>> 
>>> It may or may not be possible for our GH setup to actually prevent such 
>>> checkins (the Apache GH setup is somewhat restricted, and various things 
>>> like commit hooks and webhooks have to be configured by INFRA, not us). 
>>> 
>>> I'd like to further discuss whether people feel such a hook would be 
>>> acceptable, onerous or otherwise. Personally, I worry that such a setup 
>>> might prevent us from checking in some of the exceptions above, but if 
>>> there is a way around it, we could proceed down that path. 
>>> 
>>> What do you think, sirs?[2] 
>>> Joan 
>>> 
>>> 
>>> [1]: https://github.com/apache/couchdb/pull/753 
>>> [2]: It's a Mystery Science Theatre 3000 Joel reference. :) 
>> 

Reply via email to