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

[Zachary Turner]

Digging up this old thread because I thought of another use case that would
be nice to support.  I would like to be able to attach files generated with
git format-patch to Phabricator reviews.  I guess it chokes on the header
information though and rejects the patch as invalid.


On Wed, Jul 9, 2014 at 2:14 PM, Chandler Carruth <chandlerc at google.com>
wrote:

>
> On Sat, Jul 5, 2014 at 7:42 PM, Nick Lewycky <nicholas at mxc.ca> wrote:
>
>> Chandler Carruth wrote:
>>
>>>
>>> On Thu, Jul 3, 2014 at 11:00 PM, Nick Lewycky <nicholas at mxc.ca
>>> <mailto:nicholas at mxc.ca>> wrote:
>>>
>>>     I don't like the lack attached patch files on the mailing list to do
>>>     a normal review.
>>>
>>>
>>> Wait what? The emails I get from phab *have* an attached patch file.
>>> That was a hard requirement when we first set up Phabricator.
>>>
>>
>> Aaron nailed it. The initial emails come with attached patches. The
>> problem is when people comment with the changes they made to the code, but
>> there's no updated patch attached to that email. Aaron found examples so
>> I'll defer to those. I can also keep an eye out for the next time it
>> happens if you want.
>>
>
> Manuel is planning to look into this but is on vacation so I just wanted
> to follow up with a concrete suggestion:
>
> If you are using Phabricator (which I still think is very useful), I think
> it is important to actively look at the mailing list results. If you meant
> to update the patch and the email didn't have one attached, reply to the
> email with an attachment of the updated patch for folks to use.
>
> While I'm looking forward to improvements that fix these issues, I still
> find Phab very helpful as-is and just plan to observe and manually correct
> any bad behavior on the email thread as that is (and should always be) the
> canonical review log.
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140822/4f29dc77/attachment.html>