launchpad-dev team mailing list archive

[Christian Robottom Reis <kiko@xxxxxxxxxxxxx>]

On Wed, Aug 26, 2009 at 03:00:21PM +1000, Martin Pool wrote:
> 2009/8/26 Jonathan Lange <jml@xxxxxxxxxxxxx>:
> >> We require two people to provide +1's and no one to provide a -1
> >> before a branch can land.  Our reviews are comments on the bug
> >> report for the issue.  This mostly works well, but ambiguous
> >> situations arise, such when a +1 has been given by reviewer1 and
> >> then reviewer2 finds a major problem.  At this point the +1 from
> >> reviewer1 isn't really valid, they need to go back and re-review.
> >> We currently manage this by following the discussion.  When we don't
> >> know what the status is we talk to a reviewer directly to figure it
> >> out.
> >>
> >> Merge proposals don't really change this situation.  This isn't
> >> really a blocker as much as it's a non-adopter.
> >
> > Good point! Could you please file this as a bug too?
> 
> The question seems to be: what could Launchpad do better here, without
> requiring strong AI to interpret the comments.  Perhaps just asking
> the human, after each comment, "now what's the overall state of the
> discussion?" by putting the status in the comment field not
> separately?

Well, I don't see how the current system couldn't handle this -- if the
second reviewer finds a big problem with the patch, he would just have
to flip back the original review back to needs-review and that would be
it.

I agree that more tightly integrating the MP code review with bugs is a
great idea, but I definitely think we do much better than Bugzilla in
keeping code review comments separate from bug comments. The first thing
I'd do to help improve the situation is to make the reviews on the
MP clear from the bug page itself:

    https://bugs.edge.launchpad.net/soyuz/+bug/419147

We could have something like:

    Related branches
        _lp:~julian-edwards/launchpad/dsp-changelog-bug-419147_
            In Progress: 2 reviews, 1 abstain (_diff_)

Note the In Progress; what the hell does Development mean? ;-)
-- 
Christian Robottom Reis | [+55 16] 3376 0125 | http://launchpad.net/~kiko
                        | [+55 16] 9112 6430 | http://async.com.br/~kiko