Usability of phabricator review threads for non-phab-users

[Tobias Grosser]

On 01/07/2014 15:53, Tobias Grosser wrote:
> On 01/07/2014 13:33, Tobias Grosser wrote:
>> On 01/07/2014 13: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.
>>
>> Hi Manuel,
>>
>> thanks for looking into this.
>>
>> I remember me having issues due to patch comments missing relevant
>> context when looking at them on the mailing list. I think this has been
>> both too little code lines, but possibly also previous comments which
>> have not been cited. When people use email, they normally ensure that
>> the relevant code/comments are properly cited. Phabricator does not need
>> this in the web interface, which makes the emails sometimes less useful.
>
> Below an example of such a "less useful reply".

Another example:

> Enables "hello world" on x32 target
>
> http://reviews.llvm.org/D4181
>
> Files:
>   lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

  The commit message is not shown (or just very briefly). Later is is
  updated:

> Updating the commit message.
>
> http://reviews.llvm.org/D4181
>
> Files:
>  lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

Again, no information. On the other side, the web interface contains the
correct commit message:

This is minimal change for backend required to have "hello world"
compiled and working on x32 target (x86_64-linux-gnux32). More patches
for x32 will follow.  Note that this patch for clang is also required:
http://reviews.llvm.org/D4180

Cheers,
Tobias