Yeah, +1 to +1'ing your own PR when its trivial. Minor annoyance but its a paper trail of sorts anyway.
On Fri, Aug 18, 2017 at 10:55 AM, 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. :) >>
