Phabricator differential is not good - WAS - Re: Phabricator: All repositories registered - upcoming workflow changes

[Ben Cooksley]

On Fri, Feb 3, 2017 at 12:18 PM, Albert Astals Cid <aacid at kde.org> wrote:
> El diumenge, 29 de gener de 2017, a les 8:32:21 CET, Ben Cooksley va escriure:
>> Hi everyone,
>>
>> We've just completed the registration of all mainline repositories
>> (not including Websites or Sysadmin namespaced ones) on Phabricator.
>> Thanks go to Luigi Toscano for providing significant assistance with
>> this process.
>>
>> From this point forward, communities should be moving away from
>> Reviewboard to Phabricator for conducting code review.
>
> I just created first patch with the phabricator web interface.
>
> Found one minor and one major problem.
>
> Minor problem:
>  * You can't update the diff before creating a "Revision", so if you realize
> your diff was wrong, back luck, you either leave the diff floating in the
> limbo or you create the Revision and the update the diff, showing the world
> your mistake for no reason
> https://phabricator.kde.org/D4422?vs=10881&id=10882

Interesting. It might be worth asking upstream about that.

>
>
> Major problem:
>  * It doesn't show context
> https://phabricator.kde.org/D4422
>
> "Context not available." is terrible, how is one supposed to review without
> being able to read the rest of the code?
>
> This is a deal breaker for me.

Please see https://secure.phabricator.com/T5029

This only occurs when patches are uploaded from the web interface and
the patch in question has minimal context.
At this time Phabricator is not able to automatically resolve context
using markers in the patch (there are certain complexities involved
for some SCMs, particularly for SVN - which Phabricator supports)

The fix for this is to either:
a) Use Arcanist, the recommended tool for working with Phabricator
(this is no different to rb-tools for Reviewboard)
b) Use "diff -U99" when generating your diffs for uploading to Phabricator

In regards to improvements in Diffusion (Repository Hosting) and
Differential (Code Review) please see the upstream task at
https://secure.phabricator.com/T12010 for the work which is currently
in active progress (many of these

>
> Cheers,
>   Albert

Regards,
Ben