Strawman: eliminating debdiffs

[Colin Watson]

On Thu, Oct 09, 2008 at 08:08:34PM +0200, Daniel Holbach wrote:
> The part in our process that involves subscribing ubuntu-*-sponsors
> basically means "somebody who can upload the patch, please review it".

Right. At root, I think the thing that makes me dissatisfied with our
current process is that people who are sending us patches - things that
fundamentally need review by developers - are getting responses from
non-developers (and sometimes even told to go somewhere else by
non-developers before developers get a look-in, which is appalling). In
effect, we've set up a technical support firewall, which is always
guaranteed to annoy people who know what they're doing.

Our processes for patch handling should concentrate on funnelling them
straight through to developers in the most efficient way possible. In
theory, I can see why it's tempting for a larger group of people to try
to improve things before the small highly-contended group of developers
needs to worry about it - that's a principle we apply to bug triage work
in general - but in this case I don't think it actually helps developers
process things significantly more quickly. We do have a problem dealing
with sponsorship requests, but this doesn't seem like the solution
(except in that it might make some patch submitters give up).

> So how can we get the list of bugs with patches
> (https://bugs.launchpad.net/ubuntu/+bugs?field.has_patch=on has - 2006
> bugs right now) reasonably processed? It'd be nice if they all get
> reviews and the number of open bugs with patches down to 0.

(I see 1839 right now, so evidently somebody has been doing some good
work.)

In general I believe that anything involving a huge list of items to do
needs at least the following in order for our developers to be able to
process it effectively:

  * There needs to be a way for developers to say that they have looked
    at the item and it does not apply, without necessarily closing it
    altogether.

  * There should be a way for a developer to claim an item off the list
    to avoid duplicating work. As we've found, this works better if
    developers claim things rather than if they're assigned to them
    (where they get lost in the noise).

  * It should be concise, searchable, and ideally not divided into pages
    (so that you can use your browser's search facilities, not to
    mention the rather excellent specialised search facilities built
    into the human brain).
    https://bugs.launchpad.net/ubuntu/+bugs?field.has_patch=on is
    hopeless because it consists of 25 pages and has lots of duplicated
    bug numbers; I'm never going to skim through that looking for things
    I'm an expert on.

  * We should valiantly resist the temptation to add yet another list
    that people need to monitor regularly. I think I'm already at my
    practical limit in terms of lists I go over with my team on a weekly
    basis.

  * Some kind of incentive helps. Of course this sort of work will tend
    to accrue karma, but perhaps some way to find a list of champion
    patch integrators would help. The incentive doesn't necessarily have
    to involve your name, though; I've found that seeing the bug graphs
    at http://people.ubuntu.com/~bryce/Plots/ drop can be a great
    incentive when I'm working on a package's bugs.

  * There should be a way to get an item off the list once a developer
    has responded and it's waiting for further information. Once that
    further information is provided, it should reappear on the list,
    usually quite high up (since it is now likely that it's higher up
    the quality scale).

> Our workflow for getting some kind of patch integrated into Ubuntu is
> somewhat limited by Launchpad Bugs not offering information about the
> status of a patch. Maybe the solution is to have some kind of "patch
> triage" and a guide for how to do it. Items that come to my mind are:
>  - *is* it a patch (and not a screenshot)?
>  - does it apply against the current development version?
>  - where is the patch from?
>  - was it accepted upstream already?
>  - is it properly documented?
>  - etc.

Anything that involves judgement of the patch itself needs to be done by
people who are competent to judge it, i.e. developers. I'm afraid I've
seen far too many cases of people itching to get the bug count down for
any reason they can find rejecting perfectly good patches, and I would
be very concerned about an explicit patch triage program. I would phrase
this differently, as "what makes a good patch", but with a note that
patches that don't "comply" to the letter may well still be perfectly
useful.

> If somebody is willing to tidy up the patch, bring it into debdiff
> format, add a nice changelog entry, etc during this "patch triage" even
> better. Once this has been done, it would make sense to have some
> ubuntu-dev member review it, test it, and if it's all good upload it.

I don't object to people tidying up patches, but honestly I think there
are much better uses of their time. Trivial cleanups that anyone can do
are only going to take the developer a minute or two, and with much less
risk of error. The things that make patch review slow are not trivial
cleanups like this, but rather having to go back and forward with the
patch submitter about the intent of the patch, and the inevitable time
delays while people notice that something has been said.

Personally, it would speed my patch review up very significantly if I
could have a time-ordered list of cases where people have responded to
my comments on a patch that failed review (or even just a procmail rule
that would copy just those bug mails into my inbox or send it in the
direction of my desktop notification script). As it is, I make a comment
on a patch, the submitter replies a few days later when they have time,
and then the comment is probably lost in incoming bug mail.

> To try to workaround the limitations in Launchpad Bugs and add
> information to classify those 2006 bugs in terms of "what needs doing
> now?" we could try the following:
>  1) patch is attached and turns up on "open bugs with patches" list
>  2) patch triage happens, tag "triaged-patch" added
>  3) ubuntu-*-sponsors is subscribed, review happens
>  4) patch needs work, ubuntu-*-sponsors is unsubscribed
>  5) ubuntu-*-sponsors re-subscribed, review is good, uploaded
>  6) bug closed

If the patch triage step consisted only if "*is* it a patch (and not a
screenshot)", and also filtered out cases where a developer has already
responded giving negative feedback on the patches, I think it would make
sense for step 3) to happen en masse. I would be unhappy with any more
extensive patch triage unless it's being done by developers, in which
case you might as well just subscribe ubuntu-*-sponsors anyway.

Thanks,

-- 
Colin Watson                                       [cjwatson at ubuntu.com]