launchpad-dev team mailing list archive

[Jonathan Lange <jml@xxxxxxxxxxxxx>]

On Wed, Oct 28, 2009 at 3:19 AM, Tim Penhey <tim.penhey@xxxxxxxxxxxxx> wrote:
> Hi All,
>
> One thing that has been talked about recently is showing more of the
> interesting information about the merge proposal on the branch and bugs pages.
>
> Lets look at the branch page first.
>
> Right now looking at two branches, I see the following:
>
> For a rejected proposal:
>  Rejected for merging into lp:launchpad
>
> For one going through a review:
>  Needs review for merging into lp:launchpad/devel
>
> And an approved one:
>  Approved for merging into lp:launchpad/devel
>
> Now this is nice and all, but it doesn't really give enough information.
> Other bits of information that it would be useful to see would be:
>  * who rejected the proposal
>  * who did the reviews and what were their statuses
>  * how big is the diff
>

* How long it's been in that state for.
* The diff itself

> 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
>
> * "Rejected" links to the main proposal page and has the title text
>  "Stuart Bishop on 2009-06-17" to show who rejected it
> * Reviewers link to the user page, votes are coloured like the review table on
> the main page, review type is in brackets if it has a review type
> * The diff should hyperlink to the diff itself
>  - I'd like to have a form overlay lazy loaded with the diff if the user left
> clicks
>  - loading in a new window would take to the librarian file? or a page that
> just renders the diff?
>
> Pending reviews would show like:
>
>  Needs review for merging into lp:launchpad
>    Canonical Launchpad Engineering: Pending
>    Diff: 75 lines
>
>
>
> Now on the bugs page, we only show a branch now, and this also shows the
> status of the branch (which is next to useless - only next to useless as
> showing merged is actually useful).
>
> e.g. https://bugs.edge.launchpad.net/launchpad-code/+bug/376279
>
> Here we see:
>  lp:~mwhudson/twisted/fix-FILEXFER_ATTR_ACMODTIME  (Development)
>
> Now there is an approved merge proposal for this branch (as seen from the
> branch page).
>
>  Approved for merging into lp:~launchpad-pqm/twisted/trunk
>
> I propose that we effectively show what shows on the branch page under the
> branch link, and also remove the status (unless it is merged).
>

Agreed. Perhaps the actual logic shouldn't rely too much on the branch
status, since you can also get whether or not a branch is merged from
its merge proposal.

> 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.
>
> Any comments before I JFDI?
>

Include times. Make diffs visible inline.

jml