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 > setup?
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. Agreed :)
signature.asc
Description: PGP signature
_______________________________________________ Gluster-infra mailing list [email protected] http://www.gluster.org/mailman/listinfo/gluster-infra
