[LLVMdev] Usability of phabricator review threads for non-phab-users

[Alp Toker]

On 01/07/2014 22:53, David Blaikie wrote:
> On Tue, Jul 1, 2014 at 12:00 PM, Manuel Klimek <klimek at google.com> wrote:
>> On Tue, Jul 1, 2014 at 7:37 PM, Alp Toker <alp at nuanti.com> wrote:
>>>
>>> On 01/07/2014 21:28, Alp Toker wrote:
>>>> Specifically the problem I've been seeing is that people using the
>>>> website are unable to CC mailing list-based developers. As a result I don't
>>>> get copied in on responses to my review comments, and rarely get any kind of
>>>> direct mail with threading. You end up having to dig up historic responses
>>>> in the mailing list archive which becomes tedious.
>>>>
>>>> Often the CC on website reviews will include arbitrary names of people
>>>> who have website accounts, while excluding the actual code owners and recent
>>>> committers who you'd expect would be relevant. This leads me to guess that
>>>> the website is actively blocking the email addresses of LLVM developers from
>>>> getting added to the CC list unless they open an account on the service.
>>>
>>> To back this up, I get about a dozen mails a month saying "I can't find
>>> you on Phabricator", to which I usually reply "Just enter my committer name
>>> / email address".
>>>
>>>   AFAICT people rarely do that, or the site blocks the email address and
>>> tries to make me create an account which I'm not planning to do at present.
>>>
>>> The net result is that other people else ends up CC'ed because they do
>>> have an account on the website, and they attempt to review the code even
>>> though someone else requested the changes. At that point it becomes a matter
>>> of dealing with the fallout and things get pointlessly awkward :-/
>>
>> Btw, I'm specifically looking for others who have similar problems to Alp
>> here - pretty much all other things mentioned are on my radar.
> For the specific issue of threading, I don't find it problematic due
> to my workflow:
>
> All emails to the MLs get filtered to appropriate labels (GMail), any
> thread with an email addressed to me is starred. So if someone replies
> to a thread, but doesn't reply to me, that not a distinction my email
> rules draw - it's just a thread I'm interested in, regardless.
>
> But I totally understand that other people have other workflows that
> may be more centered around replies to /them/ personally - useful when
> you don't have time to read the full fire hose worth of LLVM mailing
> lists.

Yes, the reply-based workflow is the only one I've found to be workable 
when contributing across a lot of different projects and mailing lists.

The problems boil down to, and I guess could be resolved by implementing 
the two following recommendations:

a) List all known committer email addresses in the Phabricator CC box, 
not just website members.

A very naive Phabricator implementation might be:

1) Put this in a cron job:
   `git log --pretty=format:"%an <%ae>" | sort -u` > committers.txt

2) Load it from PHP:
   $llvm_committers = file("committers.txt", FILE_IGNORE_NEW_LINES);

3) Populate the CC list box when outputting the page from the 
$llvm_committers array.

I know it's never that simple, but I'd imagine it's not *a lot* more 
work than that?

b) Automatically add email correspondents to the CC list of a 
Phabricator thread if they reply. Optionally validate the address 
against the verified committers.txt in (a).

I suspect that'd go a long way to resolving workflow problems for 
non-members of the website.

Alp.


> Even then, I think the right workflow is usually to update the Phab
> review, then reply to the person describing the changes. It's still
> not perfect - your reply to them won't have the updated patch, it'll
> be in a separate email to the list, but I usually assume that's "close
> enough". But simply updating and saying "addressed the things in that
> other email that was sent" is a bit too disconnected - it's valuable
> to have the direct reply to the original feedback as well.
>
> - David
>
>>>
>>>
>>> Alp.
>>>
>>>
>>>> In fact as far as I can tell, mailing list-based developers are
>>>> *completely* excluded from the CC list visible on the website. This creates
>>>> a really poor workflow with responses often getting missed, and the right
>>>> people not seeing patches (and conversely, it looks like people who aren't
>>>> really relevant end up getting pressured into reviewing a patch in some
>>>> area).
>>>>
>>>> Alp.
>>>>
>>>>
>>>>
>>>> On 01/07/2014 14:11, Manuel Klimek wrote:
>>>>> Alp noted that the current setup on how phab reviews land on the list
>>>>> are not working for him. I'd be curious whether his setup is special, or
>>>>> whether there are more widespread problems. If this is more widely perceived
>>>>> as a problem, please speak up, and I'll make sure to prioritize the fixes
>>>>> (note that this is unrelated to the "lost email" problem - those are always
>>>>> highest priority and as far as I'm aware we diagnosed and fixed all of them
>>>>> within 1-2 business days).
>>>>>
>>>>> If you have the feeling that the phab email workflow makes it hard for
>>>>> you to jump into reviews, keep track of reviews, or understand reviews if
>>>>> you're not a phab user, please reply to this thread. You don't need to
>>>>> provide details, "+1", "please fix", or "doesn't work well for me" are all
>>>>> acceptable replies here - I want to get a feeling for the magnitude of the
>>>>> problem.
>>>>>
>>>>> Thanks,
>>>>> /Manuel
>>>>>
>>> --
>>> 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