El dilluns, 6 de febrer de 2017, a les 10:19:30 CET, Ben Cooksley va escriure: > On Mon, Feb 6, 2017 at 8:39 AM, Albert Astals Cid <aa...@kde.org> wrote: > > El dilluns, 6 de febrer de 2017, a les 8:18:04 CET, Ben Cooksley va escriure: > >> On Sun, Feb 5, 2017 at 6:24 AM, Albert Astals Cid <aa...@kde.org> wrote: > >> > El dissabte, 4 de febrer de 2017, a les 12:44:54 CET, Ben Cooksley va > >> > > >> > escriure: > >> >> On Sat, Feb 4, 2017 at 11:41 AM, Albert Astals Cid <aa...@kde.org> wrote: > >> >> > El divendres, 3 de febrer de 2017, a les 21:06:08 CET, Ben Cooksley > >> >> > va > >> >> > > >> >> > escriure: > >> >> >> On Fri, Feb 3, 2017 at 12:18 PM, Albert Astals Cid <aa...@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 > >> >> > > >> >> > As said on IRC, the fact that this has been open for almost 3 years > >> >> > is > >> >> > more a concern than a relief. > >> >> > >> >> I've inquired with upstream, and they've indicated that at the moment > >> >> T5029 isn't on their roadmap for implementation (although T5000 and > >> >> T182 are). > >> >> > >> >> Their target audience is primarily corporate development workflows, > >> >> for which requiring use of Arcanist isn't an issue. > >> >> > >> >> >> 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) > >> >> > > >> >> > This is not ok, the web interface for reviewboard was as good as > >> >> > rb-tools > >> >> > (i guess tbh i never used them) and "forcing" the use of a weird > >> >> > tool > >> >> > noone has heard of is not a good way to attract new contributors > >> >> > >> >> New contributors who aren't willing to install Arcanist can use diff > >> >> -U99 I would imagine? > >> > > >> > Yes, If there's an easy way for them to know they should (which afaics > >> > is > >> > not right now). > >> > >> Okay, we'll look into adding a message box or something there > >> explaining the need to use diff -U99. > > > > FWIW i thought that diff -U99 would give you the same behaviour that when > > using arc (somehow i thought phabricator was not very smart and needed > > more > > lines to actually match the patch against the whole file). > > > > But no, using diff -U99 only gives you more lines because you used -U99, > > but you can't still expand the whole file like when using arc. > > Are you essentially saying that the migration cannot proceed, because > Phabricator doesn't know how to expand patches?
I'm saying i personally think it's a severe regression and i will not be using Differential for the very few patches i create until i'm forced to, since Reviewboard is much more useful for my worflow. Obviously, given how few patches i create or review i can not say the migration can not proceed. Cheers, Albert > > I'd suggest using "diff -U99999999999999999" or some other massively > long number, if having the full file available is a hard requirement. > > > Cheers, > > > > Albert > > Regards, > Ben > > >> > Cheers, > >> > > >> > Albert > >> > >> Regards, > >> Ben > >> > >> >> > Cheers, > >> >> > > >> >> > Albert > >> >> > >> >> Regards, > >> >> Ben