Ahoy,
As part of my playing around with a webhook for custom review rules (see here for context), I noticed the following:
Description
As the author of a diff that triggers the addition of an owner package as a blocking reviewer (ie, with the settings in the image below), I’m actually free to remove the blocking reviewer and get the diff landed without input from the package owner.
I was expecting the same behaviour as for reviewers added via a herald rule: in that case, if project X was added as a blocking reviewer and I remove it from the diff, herald immediately re-adds it, as illustrated in this screenshot:
The obvious workaround is to add owners via a herald rule, though it makes the Owner package feature less useful, as each package then requires a rule that checks if that owner is involved, and adds it as a reviewer.
Reproduction Instructions
- On a repo, define an owner package (via
<your-phab-instance>/owners/that requires all changes to be reviewed in a blocking manner for paths/blocked-fooand set the owner to some group G (of which you are not a member) - Create a file
/blocked-foo/bar.txtand commit, then create a diff - Remove the blocking reviewer from the reviewer list
- Get the diff accepted by someone who is not part of group G
- Land the diff
Phabricator/Arcanist Version
phabricator 86ad69863930 Oct 19 2020|b2e96df3a3be|
arcanist |ac54d61d7af2 Oct 19 2020
Notes
The account with which I tested this is an admin account, but given phabricator’s philosophy I’m not expecting this to be a factor? If you’d like me to test with a non-admin user I’ll be happy to oblige. Non-admin accounts can remove blocking reviewing owners all the same.
Kindly,
Julien