[Gluster-devel] Gerrit review, submit type and Jenkins testing

[Michael Adam]

On 2016-01-12 at 08:46 +0530, Raghavendra Talur wrote:
> On Jan 12, 2016 3:44 AM, "Michael Adam" <obnox at samba.org> wrote:
> >
> > On 2016-01-08 at 12:03 +0530, Raghavendra Talur wrote:
> > > Top posting, this is a very old thread.
> > >
> > > Keeping in view the recent NetBSD problems and the number of bugs
> creeping
> > > in, I suggest we do these things right now:
> > >
> > > a. Change the gerrit merge type to fast forward only.
> > > As explained below in the thread, with our current setup even if both
> > > PatchA and PatchB pass regression separately when both are merged it is
> > > possible that a functional bug creeps in.
> > > This is the only solution to prevent that from happening.
> > > I will work with Kaushal to get this done.
> > >
> > > b. In Jenkins, remove gerrit trigger and make it a manual operation
> > >
> > > Too many developers use the upstream infra as a test cluster and it is
> > > *not*.
> > > It is a verification mechanism for maintainers to ensure that the patch
> > > does not cause regression.
> > >
> > > It is required that all developers run full regression on their machines
> > > before asking for reviews.
> >
> > Hmm, I am not 100% sure I would underwrite that.
> > I am coming from the Samba process, where we have exactly
> > that: A developer should have run full selftest before
> > submitting the change for review. Then after two samba
> > team developers have given their review+ (counting the
> > author), it can be pushed to our automatism that keeps
> > rebasing on current upstream and running selftest until
> > either selftest succeeds and is pushed as a fast forward
> > or selftest fails.
> >
> > The reality is that people are lazy and think they know
> > when they can skip selftest. But people are deceived and
> > overlook problems.  Hence either reviewers run into failures
> > or the automatic pre-push selftest fails. The problem
> > I see with this is that it wastes the precios time of
> > the reviewers.
> >
> > When I started contributing to Gluster, I found it to
> > be a big, big plus to have automatic regression runs
> > as a first step after submission, so that a reviewer
> > has the option to only start looking at the patch once
> > automatic tests have passed.
> >
> > I completely agree that the fast-forward-only and
> > post-review-pre-merge-regression-run approach
> > is the way to go, only this way the original problem
> > described by Talur can be avoided.
> >
> > But would it be possible to keep and even require some
> > amount of automatic pre-review test run (build and at
> > least some amount of runtimte test)?
> > It really prevents waste of time of reviewers/maintainers.
> >
> > The problem with this is of course that it can increase
> > the (real) time needed to complete a review from submission
> > until upstream merge.
> >
> > Just a few thoughts...
> >
> > Cheers - Michael
> >
> 
> We had same concern from many other maintainers. I guess it would be better
> if test runs both before and after review.

Yes. That would be ideal, imho, if the additional delay
is acceptable.

> With these changes we would have removed test runs of work
> in progress patches.

'these changes' being not running tests before review?
If test runs of WIP patches is not desired, then this
is mostly a matter of education... :-)

With the new vagrant based in-tree selftes infrastructure,
it is now really easy to run tests locally. On the other
hand, it is also very convenient to have some online
platform where one can just submit WIP patches for testing.
Maybe one could have one jenkins instance for this purpose
which uses resources separate from the review-push-jenkins?

Another 2 cents of mine..

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20160112/6a73488b/attachment.sig>