Re: PSA: pay attention when setting multiple reviewers in Phabricator

>>> A better solution would be to have in-tree metadata files providing >>> subscription rules for code review (e.g. a mapping of usernames to a list >>> of patterns matching files). Module owners would be responsible for >>> reviewing changes to these rules to ensure that automatic delegation >>>

Re: PSA: pay attention when setting multiple reviewers in Phabricator

A better solution would be to have in-tree metadata files providing subscription rules for code review (e.g. a mapping of usernames to a list of patterns matching files). Module owners would be responsible for reviewing changes to these rules to ensure that automatic delegation happens to the co

Re: PSA: pay attention when setting multiple reviewers in Phabricator

>On 05/07/2018 18:19, Mark Côté wrote: >> I sympathize with the concerns here; however, changing the default would >> be a very invasive change to Phabricator, which would not only be complex >> to implement but troublesome to maintain, as we upgrade Phabricator every >> week or two. Makes sense.

Re: PSA: pay attention when setting multiple reviewers in Phabricator

There are indeed a lot of edge cases and concerns, but this is off-topic for this thread now. We're still in early stages here, as some more groundwork has to be laid. We'll be coming back to this after some crucial Phabricator/Lando work has landed. It's something we're quite interested in and has

Re: PSA: pay attention when setting multiple reviewers in Phabricator

I sympathize with the concerns here; however, changing the default would be a very invasive change to Phabricator, which would not only be complex to implement but troublesome to maintain, as we upgrade Phabricator every week or two. This is, however, something we can address with our new custom c

Re: PSA: pay attention when setting multiple reviewers in Phabricator

On 05/07/2018 18:19, Mark Côté wrote: I sympathize with the concerns here; however, changing the default would be a very invasive change to Phabricator, which would not only be complex to implement but troublesome to maintain, as we upgrade Phabricator every week or two. This is, however, som

Re: PSA: pay attention when setting multiple reviewers in Phabricator

Also sprach Mark Côté: > On Thu, Jul 5, 2018 at 11:37 AM, Andreas Tolfsen wrote: > >> Also sprach Andrew Halberstadt: >> >>> It might be worth investigating whether we can switch Phabricator's >>> default (so that multiple reviews are all blocking, and to make >>> them non-blocking would requir

Re: PSA: pay attention when setting multiple reviewers in Phabricator

Hi On 05/07/2018 19:19, Mark Côté wrote: This is, however, something we can address with our new custom commit-series-friendly command-line tool. We are also working towards the superior solution of automatically selecting reviewers based on module owners and peers and enforcing this in Lan

Re: PSA: pay attention when setting multiple reviewers in Phabricator

Also sprach Andrew Halberstadt: > It might be worth investigating whether we can switch Phabricator's > default (so that multiple reviews are all blocking, and to make > them non-blocking would require the extra step). I agree with Andrew. Making the default “everyone explicitly marked as review

Re: PSA: pay attention when setting multiple reviewers in Phabricator

It might be worth investigating whether we can switch Phabricator's default (so that multiple reviews are all blocking, and to make them non-blocking would require the extra step). Personally I think setting multiple reviewers up on a first come first serve is disrespectful to those reviewers' time

Re: PSA: pay attention when setting multiple reviewers in Phabricator

Jean-Yves Avenard wrote on 3/7/18 6:23 am: On Mon, Jul 2, 2018 at 5:01 PM, Andreas Tolfsen > wrote: Also sprach Marco Bonardo: > When asking for review to multiple reviewers, and all of them must accept > your revision, you must mark them as blocking reviews, eit

Re: PSA: pay attention when setting multiple reviewers in Phabricator

On Mon, Jul 2, 2018 at 5:01 PM, Andreas Tolfsen wrote: > Also sprach Marco Bonardo: > > > When asking for review to multiple reviewers, and all of them must accept > > your revision, you must mark them as blocking reviews, either in the > > Phabricator ui or appending "!" at the end of the review

Re: PSA: pay attention when setting multiple reviewers in Phabricator

On Mon, Jul 2, 2018 at 5:01 PM Andreas Tolfsen wrote: > > > When asking for review to multiple reviewers, and all of them must accept > > your revision, you must mark them as blocking reviews, either in the > > Phabricator ui or appending "!" at the end of the reviewer name. > Otherwise > > it's

Re: PSA: pay attention when setting multiple reviewers in Phabricator

Also sprach Marco Bonardo: > When asking for review to multiple reviewers, and all of them must accept > your revision, you must mark them as blocking reviews, either in the > Phabricator ui or appending "!" at the end of the reviewer name. Otherwise > it's first-come-first-serve. Note that is an

PSA: pay attention when setting multiple reviewers in Phabricator

TL;DR When asking for review to multiple reviewers, and all of them must accept your revision, you must mark them as blocking reviews, either in the Phabricator ui or appending "!" at the end of the reviewer name. Otherwise it's first-come-first-serve. Full details: Standard8 asked for review to b