On Sun, Nov 20, 2016 at 4:07 PM, Barak Korren <bkorren@redhat.com> wrote:
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 makes the CI useless. To know if a patch actually passed the tests, you have to manually rebase each patch and wait for the CI - this takes up to 20 minutes on vdsm 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?
We have fast-forward in ioprocess and ovirt-imageio, and we are happy with this setting. Another advantage of fast-forward only merges is being able to submit multiple patches with *one click*. If you submit the top patch in a series, all patches are submitted. With the current setting (in vdsm), submitting a series of patches is a huge pain. Sometimes refreshing the page and submitting the next patch in the series works, but sometimes you have to rebase again the next patches in the series, and in the worst cases, you have to do several rebases in the same series. This when the entire series was already rebased properly before the submit. In vdsm we were bitten by this many times, and both Dan and me agree now that fast-forward is the only way. I don't think we need to agree on all projects for this, the whole point of having multiple project is that we don't to agree on every little detail, the project maintainer can do whatever they want. Thanks for raising this issue. Nir