launchpad-dev team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

On Wed, 28 Oct 2009 16:56:58 Andrew Bennetts wrote:
> Tim Penhey wrote:
> [...]
> 
> > I propose showing the completed and pending reviews under the line
> > summarising the proposal, and also having the approver/rejecter available
> > as tool tips.
> >
> >   Rejected for merging into lp:launchpad
> >       Jonathan Lange: Approve (code)
> >       Stuart Bishop: Reject (db)
> >       Diff: 773 lines
> 
> This sounds great to me.
> 
> In addition to the simple line count, I think I would like to see the
> added/removed line count, and the number of changed files.  e.g.:
> 
>     Diff: 773 lines (+333/-222), 5 files changed
> 
> That gives me a better feel for the complexity of a change than just a line
> count of the unified diff.

We have recently been calculating this information on the diffs.  There are 
still some cases where the parsing of the diff fails, but Aaron is pretty 
confident that we have fixed those now, so yes, we could look to show this 
information.

> So, on this page we'd see:
> >   lp:~mwhudson/twisted/fix-FILEXFER_ATTR_ACMODTIME
> >   Approved for merging into lp:~launchpad-pqm/twisted/trunk
> >     Tim Penhey: Approve
> >     Diff: 12 lines
> >
> > With the same links as on the branch page, including being able to see
> > the diff in a form overlay.
> 
> This also sounds great.
> 
> I wonder a little about bugs that go through a long series of rejected
> proposals... will listing them all be too much clutter.  Perhaps if
>  superseded proposals aren't listed that's plenty.  Anyway, we can deal
>  with this later, if it ever becomes a problem.

We'd only show the current active review for the branch.  Superseded ones 
would not show.

Tim