1475915 - when on pto bugzilla allows you to block review requests, but phabricator requests are not able to be blocked (make it possible to close review queue)

Status: NEW

(Conduit :: Phabricator, enhancement, P3)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

this is a confusing thing when someone is on PTO for a week or two, how can you ensure the patch author is aware of this so they can find another reviewer?

Comment 1

[:Gijs (he/him)]

How can we get this prioritized higher? This is a rubbish experience for everyone - patch authors who think people aren't responsive, and reviewers because they come back from PTO to a pile of reviews that they will have appeared to have ignored for weeks/months.

Comment 2

[Mark Côté [:mcote]]

We're scrambling to finish up the commit-series support since MozReview is now on its last legs.  Our next deploy, however, should have the Calendar app installed, which will allow people to at least warn others than they aren't available.

Another option is always to temporarily disable accounts on Phabricator.

Comment 3

[David Lawrence [:dkl]]

You can change your name in the profile section of your use settings to include that you are on PTO for some date range.
Click on the drop down arrow next to your username at the top right. Then Profile -> Manage -> Edit Profile. Then change your Real Name to include your PTO dates. When a user uses the UI to added you as a reviewer, the user list results will show your real name with the PTO text. The other part that is not addressed by this is when someone sets your username as a reviewer in the commit message when submitting a review from the command line. We could possibly add some extension code that looks at the reviewers real name on the back end and kicks an error if it contains something that looks like PTO type text although this would not be perfect always.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

I have never used the UI to set a reviewer, it has always been via |arc diff|.  Maybe I am an exception.  Could we have Phabricator email the patch author if the review is marked as PTO?  I am not sure we can know that by just the name, we would need a field like "block reviews" similar to what bugzilla has.

Comment 5

[:glob ✱]

(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #4)
> I have never used the UI to set a reviewer, it has always been via |arc
> diff|.  Maybe I am an exception.  Could we have Phabricator email the patch
> author if the review is marked as PTO?  I am not sure we can know that by
> just the name, we would need a field like "block reviews" similar to what
> bugzilla has.

as per comment 2, we're investigating phabricator's calendar app which exposes reviewer availability to arc.

Comment 6

[Mark Côté [:mcote]]

Finally got around to trying this.  If you create an event in the Calendar app and set yourself to "Away", a message like this will be displayed by Arcanist when someones tries to add you as a reviewer:

    All reviewers are away until Monday, Oct 8 2018. Continue anyway? [y/N]

Unfortunately, it's much less visible in the web UI, with only a red dot appearing beside the reviewer's name (the hover-over user details give more info).  I wonder if they will be improving this.  Regardless, it's better than it is now, and I imagine most reviewers are set via Arcanist/moz-phab.

... however, moz-phab will probably abort and leave your local clone in a not-terribly-great state.  More investigation to do here.

Comment 7

[Mark Côté [:mcote]]

moz-phab will actually freeze because Arcanist of this prompt.  We'll have to figure that part out first.

Comment 8

[Mark Côté [:mcote]]

moz-phab will actually freeze because of this prompt from Arcanist.  We'll sort that out in bug 1499205 after a few other fixes and improvements to moz-phab have landed.