>On Thu, Apr 26, 2018, at 12:41 AM, Mark Côté wrote: >> How we might use blocking reviewers in our workflow is still open, but >> it could be used for training up reviewers, in which case the trainee >> would be a regular reviewer and the module peer/owner would be a >> blocking reviewer. > >It's not uncommon for me to submit patches for review from multiple people >where I require all reviewers to sign off on the patch, usually because I >ask them to look at different parts of the patch. I don't think I have >ever sent a patch to get review from more than one person with the >intention of landing it if only one person OKs it. (I'd probably needinfo >people to get that kind of feedback.) So ignoring potential new workflows >like the training one you mention, I think I would exclusively use blocking >reviewers. It's good to know that Phabricator will help me avoid >accidentally landing patches when not all of my reviewers have signed off.
Agreed... but it sounds like they're not quite sure how this will be used. I'm concerned that developers may be confused and just as for review by N people, not realizing it can be landed when one of them r+'s. (Especially if the developer doesn't frequently use multiple reviewers.) At minimum, if this how it works, there will been to be clear communication about when to use this - or (better!) to switch the default to blocking, and have the unusual/more-work-to-ask-for case be any-of. Once in a blue moon I'll ask for a fast review from any of N people, then cancel the r?'s once i have 1 r+. but this is really rare, and mostly when there's a time deadline to land something ASAP (sec issue, or to get in ahead of a merge date). 99% of the time when I ask for N people, I need r+ from all of them. On the other side, I do know that the build peers (IIRC) us a shared reviewing setup, where most bugs are thrown in a pool for any of them to review. But that's not the normal workflow for any other group I know of in Mozilla, at least in Firefox. (I don't know them all, though.) -- Randell Jesup, Mozilla Corp remove "news" for personal email _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
