On 11/10/2015 03:10 AM, Raghavendra Talur wrote: > Hi, > > While trying to understand how our gerrit+jenkins setup works, I > realized of a possibility of allowing bugs to get in. > > Currently, our gerrit is setup to have cherry-pick as the submit type. > Now consider a case where: > > Dev1 sends a commit B with parent commit A(A is already merged). > Dev2 sends a commit C with parent commit A(A is already merged). > > Both the patches get +2 from Jenkins. > > Maintainer merges commit B from Dev1. > Another maintainer merges commit C from Dev2. > > If the two commits B and C changed code which had no merge conflicts but > were conflicting in logic, > then we have a master which has bugs. > > If Dev3 now sends a commit D with re-based master as parent, we have the > following cases: > > 1. If bug introduced above is not racy, we have tests always failing for > Dev3 on commit D. Tests that fail would be from components that commit B > and C changed. Dev3 has no idea on how to fix them and has to enlist > help from Dev1 and Dev2. > > 2. If bug introduced above is racy, then there is a probability that > Dev3 escapes from this trouble and someone else will bear it later. Even > if the racy code is hit and test fails, Dev3 will probably re-trigger > the tests given that they failed for a component which is not related to > his/her code and the bug stays in code longer. > > The most obvious but not practical solution to the above problem is to > change the submit type in gerrit to "fast-forward only". It would then > ensure that once commit B is merged, Dev2 has to re-base and re-run the > tests on commit C with commit B as parent, before it could be merged. It > is not practical because it will cause all patches in review to get > re-based and re-triggered whenever a patch is merged. > > A little modification to the above solution would be to > > * change submit type to fast-forward only > * don't run any jenkins job on patches till they get +2 from reviewers > * once a +2 is given, run jenkins job on patch and automatically > submit it if test passes. > * automatically rebase all patches on review with new master and mark > conflict if merge conflict arises. The overall idea looks good to me, however I'd be bit hesitant to give a +2 before seeing the regression votes until and unless the patch is pretty straight forwad. For me +1 sounds to be a good level to trigger the regression. Once the regression passes, maintainers can +2 a patch and then merge it manually. And then the 4th point follows. Thoughts? ~Atin > > As a side effect of this, Dev would now be forced to run a complete > regression on dev machine before sending a patch for review. > > Any thoughts on the above solutions or other suggestions? > > Thanks, > Raghavendra Talur > > > > > > > > > > > _______________________________________________ > Gluster-devel mailing list > Gluster-devel@xxxxxxxxxxx > http://www.gluster.org/mailman/listinfo/gluster-devel > _______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
