[LLVMdev] Phabricator and private reviews

David Blaikie
2014-06-25T18:25:11Z

As Manuel has pointed out previously - Phabricator, just like email,
can be used to conduct off-list reviews for a number of valid reasons.
The existence of Phabricator reviews that don't have the mailing lists
attached isn't, in and of itself, a bug.

Are there particular review threads you've got in mind that appear
problematic? (where a Phabricator-driven, off-list review, was used as
sign-off/authoritative for the purposes of community interaction?).
I've seen one or two cases where people send reviews and forget to CC
the mailing list (this happens with or without Phab) - that case is
not some systematic problem, all it takes is an email response (from
someone who can see the review) suggesting that the mailing list be
added to the thread. There have also been some bugs - as we've
discussed previously, which are resolved fairly swiftly once the issue
is raised. The most recent of which lead to a few hours delay in mail
from Phab to the mailing list, but was in no way catastrophic to the
smooth running of things (and was a "this thing just broke - OK, we
now see it's broken and fix it" - disabling Phab wouldn't really
help
us if we don't know that something's broken)

Essentially - if there are particular broken cases, let's look at
them. If they're easy to fix, we'll just fix them, as Manuel did for
the issue last week. If they're harder problems that may take weeks to
fix, then yes, it may make sense to disable or otherwise modify Phab
in the short term while the longer term issues are addressed.

So what are the threads/issues you're seeing currently?

- David

On Wed, Jun 25, 2014 at 10:44 AM, Alp Toker <alp at nuanti.com>
wrote:
> For whatever reason, patches posted to the Phabricator website still
aren't
> being sent to the mailing list, making it difficult for us to review them.
>
> I've raised this issue a couple of times in the last few weeks.
>
> In practice this has a detrimental effect to the development workflow
> because it means that code is being seen only by a small group of
> individuals who have web accounts. The code isn't hitting llvm-commits
or
> cfe-commits where the majority of code maintainers use the mailing lists
for
> review.
>
> At this point I think Phabricator should be disabled and patches should be
> send to the mailing lists *until* the technical issue is confirmed
resolved.
>
> It's really uncool that code is entering ToT through this back-channel
-- I
> appreciate that it might not be intentional, but every single patch that
> gets committed this way is a real problem for the project.
>
> Alp.
>
> --
> http://www.nuanti.com
> the browser experts
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev