Re: [Launchpad-users] merge request resubmission

[Jay Pipes <Jay.Pipes@xxxxxxx>]

Tim Penhey wrote:
> On Sat, 22 Aug 2009 05:13:08 Monty Taylor wrote:
> > Hey guys,
> >
> > I think there's a workflow issue with merge request resubmission.
> > Currently, if I've submitted a merge request and someone has suggested
> > it needs fixing, what I do is fix it, push, and then change the status
> > to resubmit. (This, in and of itself is non-obvious, btw, but not what
> > I'm writing about)
> >
> > The thing is, then the process is:
> >
> > Resubmit
> > Request Review
> > Add Comment
> >
> > This generates three emails, and it's three discreet steps. It seems to
> > me that, if I'm "re"submitting, then the default behavior would be to
> > request a review from whomever reviewed it before, or whomever was
> > requested to review it before. It also seems like on that confirmation
> > page for the resubmission that shows the revs that are going to be
> > resubmitted, that having the review request selection (prepopulated with
> > a sensible default) and a space to leave an optional comment would both
> > streamline the process and make certain aspects of the process more
> > obvious.
> >
> > Also, although it's not core to this, having the "resubmit"
> > functionality accessible through change status is clunky and odd. AND, I
> > would _dearly_ love an "approve it" button on the review page, so that I
> > don't have to write an approve comment and then go back through and
> > approve the request (especially since I can do both at once through the
> > email interface)
> >
> > Thanks!
> > Monty
> >
> > PS. Can you tell I spend a good amount of my day using the merge request
> > interface? :)
>
> Hi Monty,
>
> Thanks for your suggestions.  We are doing a bit of an overhaul of the review 
> page and workflows right now.  Take a look at 
> https://dev.launchpad.net/Code/Review/Plans to see what we are planning.

Hi Tim!

I would second Monty's suggestions and also add this request:

When I go to do a code review, it is typically because I receive an 
email saying that a contributor has proposed to merge a branch (the 
other avenue is me clicking the +activereviews link...).

Many times, one of the following has *already* occurred by the time I 
get to the code review:

1) Another contributor has already done a review of the proposed commits 
and the contributor has already pushed new commits addressing that review

2) The contributor has pushed additional commits because they either 
wanted to continue their work, regardless of the code review results, or 
they saw some things they wanted to change...

In these cases, it would be *extremely* useful to have a warning on the 
merge proposal page which says "Hey, reviewer!  This branch has been 
pushed to AFTER this merge proposal!"

This little warning would save me a bunch of time when I don't realize 
there are additional commits on the branch and mistakenly merge those 
commits when testing a merge proposal.

Note that this warning is *almost* like the "This merge proposal has 
been superseded by another proposal" link, but would occur even if the 
proposal has NOT been resubmitted.

I haven't looked at bzrlib or the launchpad code, but I would think 
implementing the above request would be fairly easy: just check the 
timestamp of the original merge proposal against the timestamp of the 
latest commit on the branch...

Thanks!

Jay