"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
Happened to me twice, it would be really nice if Phab would require confirmation of patches created without CCing one of the two lists, something like: "You have not CCed llvm-commits or cfe-commits, are you creating a private patch?" Yaron 2014-06-27 0:40 GMT+03:00 Justin Bogner <mail at justinbogner.com>:> "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 > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140627/cf2fa0bb/attachment.html>
On Fri, Jun 27, 2014 at 8:13 AM, Yaron Keren <yaron.keren at gmail.com> wrote:> Happened to me twice, it would be really nice if Phab would require > confirmation of patches created without CCing one of the two lists, > something like: > > "You have not CCed llvm-commits or cfe-commits, are you creating a private > patch?" >I filed https://secure.phabricator.com/T5495 Cheers, /Manuel> > Yaron > > > 2014-06-27 0:40 GMT+03:00 Justin Bogner <mail at justinbogner.com>: > >> "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 >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140627/b0f43870/attachment.html>