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

[Elvis Angelaccio]

On Fri, Feb 3, 2017 at 9:06 AM, Ben Cooksley <bcooksley at kde.org> wrote:
> 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

Would it be possible to add this info in the patch upload form? e.g.
near or in place of this sentence: "You can also paste a diff below,
or upload a file containing a diff (for example, from svn diff, git
diff or hg diff --git)."

It would not be a solution but at least more people would know that
the -U switch can be used as workaround.

>
> 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

Cheers,
Elvis