Bugzilla workflow with review

Michael Adam obnox at samba.org
Thu Nov 29 16:21:30 MST 2012

On 2012-11-29 at 10:25 -0800, Jeremy Allison wrote:
> On Thu, Nov 29, 2012 at 07:45:40AM -0500, Ira Cooper wrote:
> > IMHO:  Separate patches == Less confusion.
> > 
> > The chance that we'll get it wrong is lower, and as the branches diverge,
> > which they always do, we'll be posting 2-3 patches anyways.  (One for
> > master, one for 4.0 and one for 3.6 potentially.)
> > 
> > And what applies on master won't always apply on 4.0 which won't always
> > apply on 3.6.
> > 
> > It also makes it all "complete" when you go back into a bug and go "what
> > happened to close this bug."  You can read the actual code reviewed and all
> > the stages of review it went through, you don't have to go digging through
> > git, and have lost review history etc.
> +1 on this. The reason I attach separate patches
> for branches even though they may be identical to
> a cherry-pick is so that the bugzilla record is
> complete for what went into each branch, without
> having to refer to git history.

+1 on all the above arguments.

What is more, the reason was to reduce the work needed.
But you should verify that the patch applies cleanly to v4-0-test
anyway, so you have done the git cherry-pick already, and the step
of doing git-format patch and attaching it to the bug is minor.

One additional argument: on bugzilla, we can set review flags to
the whole bug and to attachments. If there is no attachment, we
can set it to the whole bug. But what if there are also patches
for older release branches attached. So everything is much more
clear and evident if there is a separate patch attached also
for v4-0-test.

I want to take the opportunity to make a couple of comments on
the processes of bringing patches to master and to release
branches in general because there haven been various questions
and discussions lately:

We currently have two processes that are independent in principle:

(1) the process of bringing a patch to the master branch

    For bringing a patch to master (1), we had until recently no
    rule, we just had team members their or other people's patches
    to master via autobuild. The new situation is that we are
    currently trying to do reviews (by two team members, including
    the author) on any patch that goes to master, with an
    encounragement to do non-author push. (No strict rule yet, but
    most people are participating - great!)

(2) the process of bringing a patch to a release branch

    For process (2), we have used the bugzilla model for some
    time now, with good success. The rule here is to have a
    bugreport for each patchset that should go into the release
    branch, and to have at least two positive reviews by team
    members (and no negative one), before assiging the bug to
    Karolin for picking the patch.

The first and most important point for me to make is that we
should NOT mix and confuse these two processes! In principle they
are independent.

At this very particular time, the master and v4-0-test branch
are very close. Many master patches are specifically developed as
fixes for 4.0. So it is very tempting to mix the processes
and even interpret the "Reviewed-by" tag in the master patch
as review+ on bugzilla. But this is wrong in general!
The branches will diverge (they already do), and review for
master does not necessarily mean review for any release branch.
So I think it is important to stick to that rule to do the
separate review+ in bugzilla. And it is not much work, if you
already know you want the patch in 4.0. It is just a few clicks
for the author and reviewer.

One more comment on the process of bringing a patch to 4.0 via
bugzilla: It is imho important that the patch has been reviewed
and landed in master before it is pushed to the release branch,
i.e. generally only review+ a patch for 4.0 and assign to Karolin
after it has landed in master.

Cheers - Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20121130/7c7dfe71/attachment.pgp>

More information about the samba-technical mailing list