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
setup? 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 am in favor of switching to zuul and would welcome not burdening the
regression infrastructure by running unqualified patches.
-Vijay
_______________________________________________
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra
