[Gluster-devel] Proposed change in Gerrit workflow

[Deepak C Shetty]

On 09/26/2012 09:32 PM, Anand Avati wrote:
>
>
> On Wed, Sep 26, 2012 at 2:55 AM, Vijay Bellur <vbellur at redhat.com 
> <mailto:vbellur at redhat.com>> wrote:
>
>     On 09/26/2012 02:52 PM, Deepak C Shetty wrote:
>
>         On 09/26/2012 11:41 AM, Vijay Bellur wrote:
>
>             On 09/26/2012 10:34 AM, Deepak C Shetty wrote:
>
>                 On 09/25/2012 04:13 PM, Vijay Bellur wrote:
>
>                     Hi All,
>
>                     We intend to bring the following change in our
>                     gerrit based workflow:
>
>                     - Introduce +2 and -2 for Verified in Gerrit
>                     - +2 for Verified to be necessary for merging a patch
>
>                     The intent of this proposed change is to get
>                     additional test coverage
>                     and reduce the number of regressions that can
>                     sneak by. Jenkins would
>                     continue to provide +1s for all submitted changes
>                     that pass basic
>                     smoke tests. An additional +2 would be necessary
>                     from somebody who
>                     tests the patch. Providing a +2 for Verified would
>                     be semantically
>                     similar to adding a Tested-by: tag.
>
>                 I have a basic doubt here.. How is +2 verified
>                 different than +1
>                 verified, which is currently provided by either the
>                 author or someone
>                 else or both. I assume that the Jenkins +1 verified is
>                 not the only
>                 thing that is seen by the maintainer before merging
>                 the patch, he/she
>                 should be looking at +1 verified from the author or
>                 someother person and
>                 take the decision accordingly during merge.
>
>
>             That is not the work flow model we follow currently.
>             Authors and
>             testers do not provide +1 verified usually and patches do
>             get accepted
>             with +1 verified from Jenkins. The necessary condition
>             today for
>             accepting a patch is +2 Code Review and +1 Verified. With
>             the proposed
>             change it would become +2 Code Review and +2 Verified.
>             This change
>             would mean that we will not merge patches even
>             accidentally when it
>             has been acked by Jenkins only.
>
>
>         Hmm, that would be different than the way other projects ( eg.
>         vdsm,
>         ovirt) use +1 verified. Wouldn't that cause confusion for
>         people coming
>         from different gerrit project ?
>
>
>     There are other projects which use +2 Verified too. One way or the
>     other there are bound to be confusions. This can be handled by
>     detailing the workflow clearly in our development-process document.
>
>
>
>         What happens if the user / author / tester verifying the patch
>         gives a
>         +1 ( thinking +2 is for priviledged/maintainer ) , the
>         workflow will
>         still break.
>
>
>     It will be the maintainers' and authors' responsibility to educate
>     such users and testers. Over a period of time we will reach a
>     point where education would not be necessary. Till then, good
>     documentation of this workflow and user education should provide
>     us adequate mitigation.
>
>
> Another less disruptive approach is to reconfigure jenkins to give -1 
> verified on test failure and 0 verified on success (but still make a 
> "passed" comment). The +1 verified should come outside of jenkins.

I think thats a better approach and second that. Another soln would be 
for gerrit to have "Build" section liek we have Code Review and Verifed 
sectoin.. and jenkins giving a +1/-1 for the Build success/Failure, but 
'guess changing gerrit would be long term ?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://supercolony.gluster.org/pipermail/gluster-devel/attachments/20120927/edb1a861/attachment-0001.html>