Code review comment for lp:~brian-murray/arsenal/date_last_updated

Revision history for this message
Bryce Harrington (bryce) wrote :

On Wed, Nov 30, 2011 at 02:58:24AM -0000, Brad Figg wrote:
> On 11/29/2011 06:49 PM, Bryce Harrington wrote:
> > Review: Approve
> >
> > I'm not super familiar with this script yet, but conceptually the new options sound sensible. +1 from me.
>
> I'm not sure how you bzr folks normally deal with these
> requests and reviews. I've emailed Brian separately with
> some questions/suggestions. Overall, this is a good fix.

If you go to code.launchpad.net/arsenal it shows all the branches. The
ones with the detour sign icon are ones that someone's proposed a merge
on.

  https://code.launchpad.net/arsenal/

If you click on the 'active reviews' link it shows you the merge
proposals relevant to you for reviewing.

  https://code.launchpad.net/arsenal/+activereviews

Click on the merge proposal you want to review. The green 'Diff against
target' link is useful to do a quick review of the diff. You can add a
comment and mark status to 'Approved', 'Needs Fixing', etc.

> I'd almost prefer it to be the default rather than a
> command line option but it's good either way.

I'm definitely open to trying it as the default. If it reduces our
cronjob loads against LP, that can only be a good thing.

Bryce

« Back to merge proposal