Page MenuHomePhabricator

Enable and document "WIP" workflow status in Gerrit
Closed, DeclinedPublic

Description

Currently there is no standard, easily parsable-by-machine way to mark a Gerrit change as work in progress (WIP). This means that either dashboards and queries need to be filtered by a human brain, or each developer has to amend their search strings by excluding "DO NOT MERGE", "WIP", -1 votes by the changeset owner, etc. and hope that they don't miss false negatives.

Add a new label "WIP", inspired by OpenStack's "Workflow" label. Its "neutral" value is 0 ("Ready for reviews"). If it is "voted" to -1 ("Work in progress"), the change cannot be submitted. This vote will stick with new uploads until it is changed back to 0.

For searches, this will allow Gerrit users to restrict search results by adding "label:WIP+0" to their filters.

Mailing list (wikitech-l) discussion summary from @greg: https://lists.wikimedia.org/pipermail/wikitech-l/2016-May/085611.html
How to do it from Tim L (from Sept 2015) https://lists.wikimedia.org/pipermail/wikitech-l/2015-September/083172.html :

Untested, the change would be something like:

diff --git a/project.config b/project.config
index 151eebd..93291e1 100644
--- a/project.config
+++ b/project.config
@@ -12,6 +12,7 @@
        owner = group ldap/ops
        label-Code-Review = -2..+2 group Project Owners
        label-Code-Review = -1..+1 group Registered Users
+       label-WIP = -1..+0 group Registered Users
        create = group Project Owners
        editTopicName = group Registered Users
        viewDrafts = group JenkinsBot
@@ -78,6 +79,11 @@
        value = +2 Looks good to me, approved
        copyAllScoresOnTrivialRebase = true
        copyAllScoresIfNoCodeChange = true
+[label "WIP"]
+       function = AnyWithBlock
+       value = -1 Work in progress
+       value =  0 Ready for reviews
+       copyMinScore = true
 [access "refs/meta/dashboards/*"]
        create = group Project Owners
        create = group platform-engineering

Related: T52842

Event Timeline

greg created this task.May 13 2016, 4:16 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptMay 13 2016, 4:16 PM
greg updated the task description. (Show Details)May 13 2016, 4:25 PM
Restricted Application added a subscriber: Luke081515. · View Herald TranscriptMay 13 2016, 5:03 PM

Thanks to T68283 korma.wmflabs.org currently parses "WIP" in the commit message and excludes such changesets e.g. from http://korma.wmflabs.org/browser/gerrit_review_queue.html . If we set up an explicit "Label", we need to check whether korma also needs some changes (separate task; I have not checked).

Why? Currently searching with -message:WIP to exclude them works.

JanZerebecki updated the task description. (Show Details)May 13 2016, 7:10 PM

Downside of the label is that if somebody else than the patch owner sets it the owner can not unset it.

Downside of the label is that if somebody else than the patch owner sets it the owner can not unset it.

That is an unfortunate downside. Can this be relaxed?

Why? Currently searching with -message:WIP to exclude them works.

The problem is some people used POC (proof of concept), DONOTMERGE and DONOTSUBMIT. We need something that is more obvious that can be set by any reviewer via the UI. Right now this makes it harder to find open patchsets that need a reviewer :-/

Is there even a UI in gerrit to remove or change someone else their vote? Currently the only way I know of is to remove the reviewer entirely from the patchset which removes their current votes including a CR-2. The right to do that can technically be given to everyone.

Anyway, no it doesn't make it harder. What we currently have allows you to filter and if you find one where a different wording was used, replace or add WIP (as you would need to with the label if it was only mentioned in the commit message).

greg removed a subscriber: Jdlrobson.May 13 2016, 9:21 PM

Why not standardize it though? That's basically the request here.

And, the various free text options are not easily discoverable by a new user, whereas a button/label is (is my working assumption/theory).

I won't die for this :) (especially when differential has this built in).

I won't -1 against this ;)

greg added a subscriber: Jdlrobson.May 14 2016, 3:55 AM

Why not standardize it though? That's basically the request here.
And, the various free text options are not easily discoverable by a new user, whereas a button/label is (is my working assumption/theory).
I won't die for this :) (especially when differential has this built in).

I meant to mention @Jdlrobson, not remove him, with this comment :) (was replying from my phone)

JanZerebecki added a comment.EditedMay 14 2016, 1:51 PM

The potentially upcoming gerrit.w.o update will add a tags feature, which might fulfill all requirements in T135245#2293843.

If we add a Gerrit label Workflow, we could configure Zuul/CI to prevent attempting to merge them on CR+2 (Gerrit would reject it anyway) That is T48860

jayvdb added a subscriber: jayvdb.

This proposal is selected for the developer wishlist voting round and will be added to a MediaWiki page very soon. To the subscribers, or proposer of this task: please help modify the task description: add a brief summary (10-12 lines) summarizing the problem that this proposal raises, topics discussed in the comments, and a proposed solution (if there is any yet). Remember to add a header with a title "Description," to your content. Please do so before February 5th, 12:00 pm UTC.

Tgr updated the task description. (Show Details)Feb 5 2017, 7:43 AM

Upstream gerrit have this https://gerrit-review.googlesource.com/#/c/97245/ proposal for wip status in gerrit.

Upstream have started implementing hip status in gerrit now, no more proposal but actually implementing it :)

There is a proposal at https://gerrit-review.googlesource.com/#/c/97245/ by David Ostrovsky. And he proposes patches to drop the drafts entirely.

Ref:
[OpenStack-Infra] Gerrit: Proposals for private changes and wip workflows
http://lists.openstack.org/pipermail/openstack-infra/2017-February/005193.html

There is a proposal at https://gerrit-review.googlesource.com/#/c/97245/ by David Ostrovsky. And he proposes patches to drop the drafts entirely.
Ref:
[OpenStack-Infra] Gerrit: Proposals for private changes and wip workflows
http://lists.openstack.org/pipermail/openstack-infra/2017-February/005193.html

Yep they actually have patches that are doing this. But it is being replaced with private edits which they are also implementing.

Proposing to decline this task as upstream support wip now which can be tested at http://gerrit-new.wmflabs.org/ will be in gerrit 2.15 which is a while away from being released.

demon closed this task as Declined.May 12 2017, 9:40 PM
demon added a subscriber: demon.

Declining because upstream is already working on this.