Niels de Vos
ndevos at redhat.com
Tue Sep 6 09:04:15 UTC 2016
On Mon, Sep 05, 2016 at 11:32:19PM -0400, Vijay Bellur wrote:
> On 09/04/2016 01:43 PM, Niels de Vos wrote:
> > On Fri, Sep 02, 2016 at 12:12:00PM -0400, Jeff Darcy wrote:
> > > > We already only merge after NetBSD-regression and CentOS-regression have
> > > > voted
> > > > back. All I'm changing is that you don't need to do the merge manually or do
> > > > Verified +1 for regression to run.. Zuul will run the tests after you get
> > > > Code-Review +2 and merge it for you with patches ordered correctly.
> > >
> > > The problem is that some reviewers (including myself) might not even look at
> > > a patch until it already has CentOS+1 and NetBSD+1. Reviewing code, having
> > > it fail regressions, reviewing a substantially new version, having *that*
> > > fail regressions, etc. tends to be very frustrating for both authors and
> > > reviewers. Fighting with the regression tests *prior* to review can still
> > > be very frustrating for authors, but at least it doesn't frustrate reviewers
> > > as much and doesn't contribute to author/reviewer animosity (apparently a
> > > real problem in this group) as much.
> > This is the main problem I see with this proposal too. There are quite
> > regular patches posted that break things in related regression tests.
> > Those corner cases can be very difficult to spot in code review, but
> > automated testing catches them. It is one of the reasons I prefer to do
> > code reviews for changes that are known to be (mostly) correct. Other
> > times changes (they should!) add test-cases, and these fail with the
> > initial versions...
> Would it be possible to have a workflow where verified +1 vote from the
> developer indicates that the regression tests have passed in their local
I *really* hope that is the case already!!
> If we can establish that protocol, then reviewers will have more
> confidence to look into such patches. Additionally authors will also have
> more motivation to run tests locally and fix obvious problems in our
> regression tests.
I think that local testing is always a must. It is not like it is
difficult to run the tests in a VM. I admit that I do not always run the
whole suite, but at minimal the tests for the component that is affected
by the change I'm going to post. I wait with Verified=+1 until I
actually did run the tests locally.
> I am in favor of switching to zuul and would welcome not burdening the
> regression infrastructure by running unqualified patches.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 801 bytes
Desc: not available
More information about the Gluster-infra