Feedback on recent Patchwork update

[Michael Ellerman]

On Tue, 2016-03-29 at 11:22 +0100, Finucane, Stephen wrote:

> David Miller recently posted a comprehensive review [1] of the latest
> version of Patchwork available on 'ozlabs.org' [2], and it's been a bit
> of an eye opener*. This instance of Patchwork has gone from v0.9.0 to
> v1.1.0, and the major version increase is mostly a reflection on the
> scale of a UI changes that Patchwork has undergone. It seems to be
> these UI changes, rather than the other non-visible changes in v1.1.0
> [3], that have irked power users like David and (presumably) others.

So I'm another "power user" of the ozlabs.org instance.

I don't process as many patches as Dave, but I do spend a significant portion
of my week interacting with patchwork.

I'll reply below to the individual points.

> > A couple of complaints about the new patchwork installed this weekend
> > on ozlabs.org, I think it's a lesson on what matters for high
> > frequency workflows for maintainers.
> > 
> > 1) The "delegate" radio button now lists all patchwork users on the
> > entire site, instead of just the users who are members of the
> > project. So instead of having to choose between 2 or 3 developers
> > for delegating a networking patch, I have to scroll through hundreds.
> > This is unusable.
> 
> This was a feature that was requested both via the mailing list and,
> funnily enough, from a member of the audience during a talk I gave
> at FOSDEM this year. We've tried to keep things as uncluttered as
> they can be through use of an auto-complete widget, though it's
> clearly not good enough. I see two options here:
> 
> * Instead of displaying all users alphabetically, the list will display
>   users who are maintainers first followed by all users.

This seems like an OK idea, except that on ozlabs.org we have a *lot* of
users. Before we reverted this change the list of "option"s in the HTML form
listing all users for the delegate drop-down was ~70KB.

My preference would be that only maintainers *appear* in the list, but that if
I type in a users name I can then assign to them.

I think that would work well in the general case, the only drawback is that
it's not very discoverable that you can type an arbitrary user's name.

> > 2) Don't change the layout of the patch list pages with new fonts,
> > new spacing, etc. when part of the usage is clicking a lot of check
> > boxes. Now all of the boxes are spaced apart differently thus
> > throwing off all the muscle memory heavy users of patchwork have been
> > accumulating over the past decade of patchwork usage.
> > 
> > Nobody who uses this stuff a lot cares if the fonts or CSS layout
> > layout is pretty, we just want to get as much work done as fast as we
> > possibly can.  And any new layout like this works against that goal.

Personally I'm not so bothered about the UI changes.

In fact I already run some custom CSS (using a browser plugin) on patchwork,
because I want it to look a certain way. So I spent 1 minute porting that CSS
to the new layout and I'm happy.

Given that I look at patchwork a lot, I do actually want it to look vaguely
"nice", and on the whole I think the redesigned UI is more pleasant than the
old one. The fonts on the old UI were really quite small, and didn't scale at
all well on a 4K display for example.

> I think the recently merged "shift-select" patch will help [4]. To
> summarise, this lets you use the shift key to select a range of
> patches and is, to me, a clear usability boost.

Yeah that would be awesome.

I'm also *really* looking forward to Series support, as the most tedious part
of interacting with patchwork is clicking 20 checkboxes to select a whole
series. In reality I rarely actually do that, because I have terrible scripts
to do it for me via the XML RPC API, but Series support should make those
obsolete.

> > 3) Comments are now after the PATCH. This is nuts. That's the first
> > thing I want to see after the commit message! I want to see if people
> > have reservations or major feedback about the patch before I even
> > see the patch itself.
> > 
> > Do people understand this? The comments and feedback are more
> > important than the patch itself. For example, if the kbuild robot
> > says ANYTHING about a patch, I'm marking it as needing changes. I
> > don't need or want to invest the time reading the patch at all when
> > this happens.  But now I have to spend time scrolling past it, this
> > is bad.
> 
> Jeremy has resolved this, and the ozlabs instance is now updated
> to reflect this.

Personally I prefer the "old" layout (change log, comments, patch), but I think
it's 50/50 on which is the right ordering. So I think this should probably just
be a user preference.

> We could also add an counter on the patch list page to state how many
> comments a patch has, if this would help?

Yes that would be awesome. When I'm skimming the list of patches one of the key
things I want to know is whether a patch has generated a lot of discussion.


The one outstanding "bug" in my opinion is that there's no longer any way to
filter for patches that have *no* delegate.

For example I have a bookmark:

  https://patchwork.ozlabs.org/project/linuxppc-dev/list/?state=1&delegate=-

Which used to show me new patches which had not been delegated.

This no longer works. It now shows me patches delegated to a user literally
called "-", which doesn't exist.

AFAICS this was lost (inadvertently) in f439f5414206 ("Add delegate filter
autocomplete support").

Reinstating this, either using "-" or "none" or "nobody" would make my week.

cheers