Comment 7 for bug 506018

Revision history for this message
Bryce Harrington (bryce) wrote : Re: [Bug 506018] Re: Need a "+patches" view: report lists patches attached to bugs.

On Thu, Jan 14, 2010 at 03:21:03PM -0000, Karl Fogel wrote:
> Bryce, is your mockup at
> http://www2.bryceharrington.org:8080/X/Reports/patches.html to be
> considered basically the way you'd want this to work? I.e., is that
> just a rough idea, or is it actually exactly what you'd like to see?

Pretty much. I would like to have a direct link to the patch itself,
but this is hard to do with launchpadlib.

Maybe linkify Package too.

I've pondered ideas on filtering the data down a bit. E.g. excluding
fix committed, incomplete w/out response, patches posted by the package
maintainer. But each of these ideas has an issue I'm not 100% sure
about.

> One thing I'm concerned about is that it's not clear how to handle bugs
> that have multiple patches attached. Your display is organized by
> bugtask-per-patch, that is, it shows every bugtask for each bug that has
> a patch attachment. If a bug had 2 patch attachments, would it display
> 2 * num_bugtasks lines? You can see how this could get out of hand
> quickly :-).

Agreed about the multiple bugtask issue, which I run into with all my
cross-package reports. It's a hard problem since if you decide to just
do one bug per line (rather than bugtask), then it breaks sorting by
packagename.

I handwave it by saying if a bugtask is filed against multiple of my
packages it's either wrong (and should be fixed), or deservedly that
much more important. (As an example, after seeing how bug #258038 was
marked against 6 of my packages, I put it on the todo list for UDS and
we are now undertaking the hard work of implementing it in Lucid.)

Regarding multiple patches per bug, what I've done in this report is
ignore all but the most recently posted. Here's my reasoning. In most
cases where I've seen multiple patches, the prior patches were just
earlier variants of the same patch, and only the latest one was relevant
for review/upload. In cases where there were multiple patches to deal
with, that would be clear from the bug's context and more than one entry
in this table would be unnecessary.

One idea for displaying multiple patches might be to display the patch
icon multiple times on the line, one for each patch.

> I'll attach a screenshot that shows what's running in the branch right
> now. Note it displays the patch attachment's message as well as the bug
> summary. Of course, that makes things rather wide; a better solution
> might be to put the patch message and/or filenmae as a tooltip-style
> text popup for when one hovers over something that identifies the patch.
> But currently, there isn't really anything that identifies the patch
> itself, except the patch age column.

Heh, in an earlier version of the patch report I also included the patch
title. But like you I found it took up way too much space. Also, I
found in practice I really didn't care about what the patch was named,
and I removed it from the list in favor of the bug title. Here's the
reasoning I used: A) As a triager/developer my brain is wired more to
recognize the bug title; B) too often users select names for patches
that aren't very helpful ("patch" or "package-1.2.3-4ubuntu5.debdiff");
C) If an update to the patch is uploaded, it will have a new name, but
the bug title stays consistent.

I also experimented with including the patch author, which actually
would be more interesting than the patch name. (I often think of
patches as "that patch from Foo Bar to fix that nasty Baz bug.")
But that also consumed more space than was useful.

I really like the idea of a popup with extra info about the patch. A
tooltip with the patch name and author would be very sweet. Have you
used netflix? Netflix has a hover-over ajaxy thingee that pops up a
mini-page showing the movie poster, brief summary, and actors. I
imagine something like that for when you hover-over the patch icons,
which displays the patch name, author, and a snippet of the patch
itself.

Bryce