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
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-users
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-users
More help : https://help.launchpad.net/ListHelp
