[ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Martin Perina]

On Sun, Nov 20, 2016 at 9:18 PM, Sandro Bonazzola <sbonazzo at redhat.com>
wrote:

> Il 20/Nov/2016 15:08, "Barak Korren" <bkorren at redhat.com> ha scritto:
> >
> > Hi there,
> >
> > I would like to address a concernt that had been raised to us by
> > multiple developers, and reach an agreement on how (and if)  to remedy
> > it.
> >
> > Lets assume the following situation:
> > We have a Git repo in Gerrit with top commit C0 in master.
> > On time t0 developers Alice and Bob push patches P1 and P2 respectively
> > to master so that we end up with the following situation in git:
> > C0 <= P1 (this is Alice`s patch)
> > C0 <= P2 (this is Bob`s patch)
> >
> > On time t1 CI runs for both patches checking the code as it looks for
> > each patch. Lets assume CI is successful for both.
> >
> > On time t2 Alice submits her patch and Gerrit merges it, resulting in
> > the following situation in master:
> > C0 <= P1
> >
> > On time t2 Bob submits his patch. Gerrit, seeing master has changed,
> > re-bases the patch and merges it, the resulting situation (If the
> > rebase is successful) is:
> > C0 <= P1 <= P2
> >
> > This means that the resulting code was never tested in CI. This, in
> > turn, causes various failures to show up post-merge despite having
> > pre-merge CI run successfully.
> >
> > This situation is a result of the way our repos are currently
> > configured. Most repos ATM are configured with the "Rebase If
> > Necessary" submit type. This means that Gerrit tries to automatically
> > rebase patches as mentioned in t2 above.
> >
> > We could, instead, configure the repos to use the "Fast Forward Only"
> > submit type. In that case, when Bob submits on t2, Gerrit refuses to
> > merge and asks Bob to rebase (While offering a convenient button to do
> > it). When he does, a new patch set gets pushed, and subsequently
> > checked by CI.
> >
> > I recommend we switch all projects to use the "Fast Forward Only" submit
> type.
> >
> > Thoughts? Concerns?
>

​AFAIR this was enabled for ovirt-engine project in the past and it was
pretty impossible to merge any patch with CI+1 when some important dates
were near (like feature freeze), because all maintainer tried to merge
patches and waited for CI to finish. Personally I'd say that current status
is OK, because it's a responsibility of a maintainer to check CI results of
a patch that he/she merged (and if error is raised then investigate the
issue and post a fix asap if needed).

So "Fast Forward Only" could successfully works for smaller projects, but I
don't think it will work for big projects like engine or vdsm.

+1 for me
>
> >
> > --
> > Barak Korren
> > bkorren at redhat.com
> > RHEV-CI Team
> > _______________________________________________
> > Devel mailing list
> > Devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
>
> _______________________________________________
> Devel mailing list
> Devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20161120/5c5c9dab/attachment-0001.html>