Re: Proposed change in Gerrit workflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/27/2012 10:55 AM, Deepak C Shetty wrote:
On 09/26/2012 09:32 PM, Anand Avati wrote:


On Wed, Sep 26, 2012 at 2:55 AM, Vijay Bellur <vbellur@xxxxxxxxxx
<mailto:vbellur@xxxxxxxxxx>> 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 ?

Yes, changing gerrit would be long term.

Even, I like the idea of Jenkins providing 0 and -1. It seems less disruptive too.

-Vijay



[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux