Page MenuHomePhabricator

No way to mark patches as not WIP (which blocks merging)
Closed, ResolvedPublic

Description

I can mark patches as WIP but not as not WIP. What's worse a bunch of patches seem to have ended up as WIP without anyone (I know of) explicitly marking them. This is pretty bad as zuul seems to refuse merging WIP patches even if they are +2-d (also they are hidden from some dashboards and notifications).

@Addshore says he has a "start review" button to un-WIP so probably it has to do with gerrit permissions.

Event Timeline

Tgr triaged this task as Unbreak Now! priority.Jun 14 2018, 1:33 PM

I got @Dzahn to fix our patches for now.
I guess as a gerrit admin he can see this button on everyone's patches.

I'm in the gerrit repo creators group, which i guess means I can for some reason see the button for my patches?

And then it just doesn't exist for anyone else...

Addshore renamed this task from No way to mark patches as not WIP to No way to mark patches as not WIP (which blocks merging).Jun 14 2018, 1:54 PM

There is a start review button in polygerrit only the owner of the change can do it.

Upstream have changed this so project owners can unmark wips.

Most changeset owners do not have that button currently, and many patches have been marked as WIP somehow (compare 437064, where I tested the WIP button, and it has a Set Work In Progress comment, with 421433 where there are no such comments), so an unknown number of developers are now unable to merge their patches without the help of a gerrit admin. (And it's not particularly easy to figure out what's going on, either; especially on the old interface which just says "Cannot Merge" like it does on patches that do not rebase cleanly.)

Patches you create directly on the Gerrit UI are marked WIP as default, and only the owner of the patch can unmark it as WIP by clicking on the 'Start Review' button. I am not aware if Admins can do this but, as Addshore, I am a Gerrit Manager and I can only see that button for the patches I own (as expected afaik).

Tgr lowered the priority of this task from Unbreak Now! to High.Jun 14 2018, 3:06 PM

I was slightly confused: the unWIP option seems to be missing completely from the new UI; on the old UI changeset owners can indeed see it. So this is not that bad a problem. Should block T196812: Make PolyGerrit the default ui in Gerrit 2.15 though, IMO.

on the old UI changeset owners can indeed see it

Although @daniel says he doesn't. Maybe there is some difference in what permissions are granted by the wmf vs. the wmde group?

I was slightly confused: the unWIP option seems to be missing completely from the new UI; on the old UI changeset owners can indeed see it. So this is not that bad a problem. Should block T196812: Make PolyGerrit the default ui in Gerrit 2.15 though, IMO.

This is not a issue with polygerrit though, it shows for me at least when looking at my wip (which some were converted from draft to wip)

https://gerrit-review.googlesource.com/c/gerrit/+/183851 is the fix which expands it to project owners too so not limited to just admins.

Sorry. It seems me and others who reported this have just missed the button on the new interface (where it replaces the "Reply" button, with the same hard-to-read color, so it's somewhat hard to spot). So owners can unWIP on both interfaces although the UX on polygerrit is extra poor.

Closing as resolved as 2.15.3 should have resolved most of these issues with only the admin being able to unmark wips :)