WIP diff revisions

We have users who want to make diffs are in Work In Progress (WIP) state and avoid those diffs kicking off builds to test them (the idea is not disimilar way to https://docs.gitlab.com/ee/user/project/merge_requests/work_in_progress_merge_requests.html ). We also want to ban (or strongly) warn on attempts to land WIP diffs.

Our existing herald rules were modified to search WIP in the diff title via regex and if found then NOT kick off builds. Another rule was added that adds a “reviewblocker” bot to diffs that have WIP in their title. This made users so happy they immediately flocked to the new “feature” only to get to the stage where they were happy with the diff and wanted to remove its WIP state:

  1. Users removed WIP from the title but found reviewblocker reviewer was still present (I’m not aware of a way to remove a reviewer via a herald rule).
  2. Even after removing reviewblocker reviewer and changing the title no builds were kicked off. It turns out that an arc diff of an empty commit was required to cause all the herald rules to be reevaluated and for those builds to be initiated.

Is there anything in current Phabricator that makes this idea of a WIP workflow a bit smoother? The draft state (https://secure.phabricator.com/T2543 ) is somehow different again because we like it as it is - we still want builds to happen on things in draft and for things to only transition out of draft when the build has completed successfully.

Why don’t you want to run builds on “WIP” revisions? They may fail if the change is in a very rough state, but it’s not obvious to me why that’s a problem: knowing that the builds don’t pass yet seems like it is still informative to reviewers.

It’s painful to admit but from my perspective it’s because the builds take a long time to finish (and there are lot of them). The developers say it’s because they like the security of seeing their changes safely backed up up, it makes it sharing nicer because comments can be left (even if it’s not actually passing the tests) but is easier because things half way done go straight to review without being judged until they are ready (and the name prepares others for what to expect). In fact, as I type this I can hear developer one telling developer two about the WIP feature and its lightweight benefits (I’m hoping developer one remains as enthusiastic when they get to the final stage but I don’t want to dampen their enthusiasm until I’m sure there’s no sneaky way out).

Okay, so it sounds like the problem isn’t really that builds run, but that the changes wait for builds to complete before sending email, and you want “WIP” changes to send email normally?

As a possible approach, you can change the behavior for slow builds from “Hold Drafts: Always” (the default) to “Hold Drafts: Never”. This will make changes promote from “Draft” immediately, without waiting for builds to complete.

Revisions will still run builds, and reviewers will be warned in the UI that a revision has ongoing or failed builds before they “Accept”, and authors will be prompted to confirm that they want to continue by arc land. If you want, you can use a Herald “Revision has build warning” rule to completely prevent authors from pushing these changes.

The cost of this is that non-WIP revisions won’t wait for builds to pass either, but maybe that’s okay in your environment, particularly if your tests routinely take a very long time to run.

It sounds like the only behavioral difference you really want between “WIP” and “non-WIP” changes is to stop users from pushing changes which have [WIP] in the commit message.

You can do this most directly with a Herald rule like this:

Global rule for Commit Hook: Commit Content
When:
[ Commit message ][ contains ][ "[WIP]" ]
Take actions:
[ Block push with message ][ You aren't allowed to push "[WIP]" changes. ]