1472689 - All the reviewers must accept the revision

Status: RESOLVED WORKSFORME

(Conduit :: Phabricator, enhancement)

Description

[Marco Bonardo [:mak]]

This looks like not yet supported by Phabricator (https://secure.phabricator.com/T10574) but it's a very common use-case in our codebase, we ask for review to multiple reviewers, one review is not enough to land.

Currently when any of the reviewers accepts the revision, the change is approved and moves to "Waiting for author", disappearing from the review queue of the other reviewers.

For example in bug 1446325 Mark asked for review to me and Lina, she accepted the revision, and my review request disappeared, it's not on the bugzilla dashboard, and in Phabricator it's under Waiting on Authors instead of being under Ready to Review.

This is pretty much dataloss for us.

Comment 1

[Mark Banner (:standard8)]

Just to expand on the use-case: This patch covers two different domains, and hence needed review from both. Unfortunately it would have been very difficult to split the patch into two, it would have also most likely caused test failures on the intermediate revision, so it wasn't worth it.

Marco only realised it was outstanding still after a discussion on irc that happened to reference it.

Comment 2

[:glob 🎈]

you can achieve this by adding each reviewer as a "blocking" review - either via the UI, or by appending ! to their name on the CLI.

you can see an example of this on https://phabricator-dev.allizom.org/D470
i added both reviewers as blocking, then approved it as glob-test.
the diff is still showing "needs review".