[LLVMdev] Phabricator and private reviews

[Alp Toker]

On 25/06/2014 21:25, David Blaikie wrote:
> 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?

Thanks, that's a brilliant question :-)

Today's concern was "Re: [PATCH] cmake: NDEBUG needlessly defined in 
non-Release builds" which Reid somehow saw and replied to but still 
isn't on any of the LLVM public mailing list archives.

I'm coordinating the 3.5 header/feature macro cleanup goal and really 
need to be in the loop on review requests like this. In fact these 
changes affect the whole source tree, so the mailing list is the ideally 
the place for review.

Alp.


>
> - 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

-- 
http://www.nuanti.com
the browser experts