couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick North <nort...@gmail.com>
Subject Re: [DISCUSSION] Disallow all merges of PRs to master that cause tests to fail
Date Wed, 16 Aug 2017 11:29:02 GMT
+1. Having put all the effort in to to get to a clean test suite, it would
be a great shame to let it slip. The effort to remain clean is much less
than the effort to get to that state in the first place: failing tests are
easier to fix when they first occur and everything is fresh in the author's
mind, than they are 6 months later. If you can't merge to master until the
tests are clean, there is a much greater incentive to take the high road
and fix problems immediately.

As a second point: if you come to an interesting Open Source project you've
never seen before, then a failing build or failing test suite is a good
reason to reject it without further thought. Cleanliness at that level is
an indication of a well-functioning and professional community, whose
software is worthy of further consideration.

Nick

On Wed, 16 Aug 2017 at 08:43 Dale Harvey <dale@arandomurl.com> wrote:

> +1, have found this useful / necessary with PouchDB over the years
>
> Describing our workflow as it may be useful (its also very similiar to
> mozillas). We have Tier 1
> supported platform, node stable, stable releases of browsers etc a checkin
> is not allowed to make
> those go red, if something lands that makes them red it gets immediately
> reverted
>
> Tier 2 systems, like CouchDB 2.0 for the period it was under development.
> We take a best effort
> at keeping it green but it doesnt prevent patches from landing. If it goes
> red it is someones job to get
> it green, if it stays red for a long period of time it becomes unsupported.
> (We are still in the process of moving
> couch2 to tier1)
>
> This allows us to stay pretty stable while not be too restrictive for
> building new features / supporting new
> platforms. The important thing however is the zero policy rule on Tier 1,
> once there is any reason for
> people to ignore failing tests they decay super quickly and it gets very
> hard to bring them back, for a while
> we had the policy of fixing things in master but there are always
> complications, backing out, fixing the tests
> and relanding is far more reliable
>
> On 16 August 2017 at 07:46, Joan Touzet <wohali@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. :)
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message