[LLVMdev] Phabricator and private reviews

[Justin Bogner]

"Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
>> On 2014-Jun-25, at 12:32, Alp Toker <alp at nuanti.com> wrote:
>> 
>> As I understand, some people legitimately use Phabricator for
>> internal review,
>
> Is this is a use case we need to support on <http://reviews.llvm.org>?
>
>> while others *think* they're submitting public patches but the
>> system doesn't forward them to the reviews lists
>
> Aside from temporary infrastructure failures, which Manuel responds to
> quickly, there seem to be two main reasons patches (and comments)
> don't get sent to the list:
>
>  1. Reviewer neglects to add llvm-commits (or adds it after the fact,
>     and the patch isn't sent).
>
>     This seems easy to fix by requiring llvm-commits on llvm patches
>     (and cfe-commits on clang patches).  Is there a technical hurdle
>     to requiring this?

+1. This is the situation I've seen most often, and while innocuous it
can be pretty annoying.

>  2. Reviewer (or commenter) has an invalid or un-subscribed email on
>     Phab.
>
>     Is it technically feasible to disallow reviews (or comments on a
>     review) from logins that aren't subscribed to the appropriate list
>     and/or have invalid email addresses?
>
> I suspect designing away these failures would fix most of the
> problems.
>
> Since the purpose of this Phab instance is to support on-list reviews,
> we should make it hard to get that wrong (even if it means making
> off-list reviews harder).
>
> -- dpnes