[LLVMdev] Usability of phabricator review threads for non-phab-users
zturner at google.com
Fri Aug 22 11:27:34 PDT 2014
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>
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits