llvm dev - Jan 2020 - Proposing a llvm-patch helper script in-tree to create/apply patches without arc

I'd rather we decided on whether to accept GitHub PRs or not first (in the
other thread). I would bet that everyone who has troubles with arc / is not
allowed to use arc would happily use GitHub PRs instead.

Worst case scenario if the community decides that we don't want to accept
GitHub PRs then this sort of script would be a useful time sink.

Cheers,
-Neil.

On Tue, Jan 21, 2020 at 9:19 AM James Henderson via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> I like the concept, and a quick glance at the script is essentially what
> I'd expect, though I haven't looked at it in depth. I'm quite
okay with
> jumping through the current hoops needed to do those three items, but they
> are also my most common operations, so a script would definitely simplify
> things for me. Unfortunately, I don't currently have the time to invest
in
> it myself.
>
> On Tue, 21 Jan 2020 at 02:16, Florian Hahn via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi,
>>
>> One takeaway for me from the recent Phabricator vs Github PR
discussions
>> was that arc (arcanist) can be a pain to set up and may pose a hurdle
for
>> some contributors.
>>
>> I think those points could be addressed relatively easily by adding a
>> llvm-patch script (or an even better name) that allows users to create
and
>> apply patches from reviews.llvm.org using Phabricators API. In my
>> experience, the three most common uses cases are:
>>
>> 1. Create a new review from the current HEAD commit in the working
>> directory and let me optionally add subscribers/reviewers
>> 2. Update the diff for an existing review with the current HEAD commit
in
>> the working directory
>> 3. Download the latest diff for a revision and apply it to the working
>> directory, using the commit message from Phabricator.
>>
>> Those should be fairly easy to implement and as a proof-of-concept I
went
>> ahead and put up a patch implementing 3. from the list above:
>> https://reviews.llvm.org/D73075 .
>>
>>  Please note that the script is probably a bit rough around the edges
and
>> I probably won’t have time to implement 1. and 2. in the near future on
my
>> own, but maybe someone would be interested in helping out.
>>
>> I think that could improve the experience for new contributors
>> substantially:
>>  * Create a new patch: `llvm-patch upload`
>>  * Apply a patch from a review: `llvm-patch apply D12345`.
>>
>> WDYT?
>>
>> Cheers,
>> Florian
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
Neil Henning
Senior Software Engineer Compiler
unity.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200121/cedfe1a6/attachment.html>

On Tue, Jan 21, 2020 at 11:54 AM Neil Henning via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> I'd rather we decided on whether to accept GitHub PRs or not first (in
the
> other thread). I would bet that everyone who has troubles with arc / is not
> allowed to use arc would happily use GitHub PRs instead.
>
> Worst case scenario if the community decides that we don't want to
accept
> GitHub PRs then this sort of script would be a useful time sink.
>
I think it is useful in that it shows good-will and a way forward for
fixing the issues that people seem to have with Phabricator.

I have yet to see similar initiative among those who favor GitHub PRs. That
may be in part because one of the main problems of GitHub *cannot* be fixed
by outside parties, but only by GitHub themselves. Being beholden to an
external party like that is a bad thing, and was in fact the reason for
creating Git in the first place...

Cheers,
Nicolai


> Cheers,
> -Neil.
>
> On Tue, Jan 21, 2020 at 9:19 AM James Henderson via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> I like the concept, and a quick glance at the script is essentially
what
>> I'd expect, though I haven't looked at it in depth. I'm
quite okay with
>> jumping through the current hoops needed to do those three items, but
they
>> are also my most common operations, so a script would definitely
simplify
>> things for me. Unfortunately, I don't currently have the time to
invest in
>> it myself.
>>
>> On Tue, 21 Jan 2020 at 02:16, Florian Hahn via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Hi,
>>>
>>> One takeaway for me from the recent Phabricator vs Github PR
discussions
>>> was that arc (arcanist) can be a pain to set up and may pose a
hurdle for
>>> some contributors.
>>>
>>> I think those points could be addressed relatively easily by adding
a
>>> llvm-patch script (or an even better name) that allows users to
create and
>>> apply patches from reviews.llvm.org using Phabricators API. In my
>>> experience, the three most common uses cases are:
>>>
>>> 1. Create a new review from the current HEAD commit in the working
>>> directory and let me optionally add subscribers/reviewers
>>> 2. Update the diff for an existing review with the current HEAD
commit
>>> in the working directory
>>> 3. Download the latest diff for a revision and apply it to the
working
>>> directory, using the commit message from Phabricator.
>>>
>>> Those should be fairly easy to implement and as a proof-of-concept
I
>>> went ahead and put up a patch implementing 3. from the list above:
>>> https://reviews.llvm.org/D73075 .
>>>
>>>  Please note that the script is probably a bit rough around the
edges
>>> and I probably won’t have time to implement 1. and 2. in the near
future on
>>> my own, but maybe someone would be interested in helping out.
>>>
>>> I think that could improve the experience for new contributors
>>> substantially:
>>>  * Create a new patch: `llvm-patch upload`
>>>  * Apply a patch from a review: `llvm-patch apply D12345`.
>>>
>>> WDYT?
>>>
>>> Cheers,
>>> Florian
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
>
> --
> Neil Henning
> Senior Software Engineer Compiler
> unity.com
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200121/2fd9c765/attachment.html>

> On 21 Jan 2020, at 02:59, Nicolai Hähnle via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> On Tue, Jan 21, 2020 at 11:54 AM Neil Henning via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> I'd rather we decided on whether to accept GitHub PRs or not first (in
the other thread). I would bet that everyone who has troubles with arc / is not
allowed to use arc would happily use GitHub PRs instead.
> 
> Worst case scenario if the community decides that we don't want to
accept GitHub PRs then this sort of script would be a useful time sink.
> 
> I think it is useful in that it shows good-will and a way forward for
fixing the issues that people seem to have with Phabricator.
> 
This was indeed my intention. Mainly I wanted to show that we could provide a
very streamlined way for basic patch management git <-> Phabricator
(single command for both patch creation and applying a patch) in tree without
too much effort.

Cheers,
Florian
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200123/5488df84/attachment-0001.html>