← Back to team overview

openstack team mailing list archive

Re: Working on fixing code after a review? Please mark merge proposal Work In Progress!


Good call Jay - I'll do my best to remember to do this!

Why are branches with unmerged pre-reqs showing up in that list?  If
reviewers are working from that list, that just seems to be creating extra
work, which you probably don't need!


On Fri, Feb 25, 2011 at 10:07 AM, Jay Pipes <jaypipes@xxxxxxxxx> wrote:

> Hey all,
> The backlog on code reviews continues to mount:
> https://code.launchpad.net/nova/+activereviews
> One thing that would REALLY help reviewers is the following:
> If you receive one or more reviews that have asked for fixes to your
> branch (Needs Fixing), and you agree to these fixes, please do the
> following:
> * Write a quick comment on the merge proposal acknowledging the review
> and stating you are working on fixing the branch
> * Set the merge proposal status to Work In Progress
> When you do this, your branch gets removed from the list of merge
> proposals that reviewers need to review (the link above), which helps
> reviewers to identify which branches are truly in need of review and
> which branches are actually being worked on after reviews.
> After you make the changes to your branch, please do the following:
> * bzr push your changes to Launchpad
> * Go to your merge proposal and set the status back to Needs Review
> This will trigger:
> * Launchpad to regenerate the diff that is displayed for your branch
> * Launchpad to email all prior reviewers to let them know they need to
> re-review your branch
> Doing these simple steps will make reviewing dramatically easier; the
> side-effect of that is reviews will be quicker and we can get through
> this backlog.
> Thanks for listening,
> jay
> _______________________________________________
> Mailing list: https://launchpad.net/~openstack
> Post to     : openstack@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openstack
> More help   : https://help.launchpad.net/ListHelp

Follow ups