llvm dev - Mar 2020 - Allowing PRs on GitHub for some subprojects

> On Mar 3, 2020, at 18:48, Eric Christopher <echristo at gmail.com>
wrote:
> 
> I'm one of those people ;)
That's not something to be proud of if you expect a maintainer to commit on
your behalf. If you commit yourself, then whatever.

Louis
> 
> -eric
> 
> On Tue, Mar 3, 2020 at 2:20 PM Louis Dionne via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> 
> 
> > On Mar 3, 2020, at 17:16, Shoaib Meenai <smeenai at fb.com
<mailto:smeenai at fb.com>> wrote:
> > 
> > `arc patch` should preserve the author information in the original
commit, if there was any. At least it has in my experience.
> 
> Yes, but I think people can upload raw patches to Phabricator without using
`arc diff`. I know I ran into one of these just last week where I used
Johannes' script (thanks!) and ended up still having to find the
committer's email by other means.
> 
> Louis
> 
> > 
> > On 3/3/20, 1:44 PM, "llvm-dev on behalf of Louis Dionne via
llvm-dev" <llvm-dev-bounces at lists.llvm.org
<mailto:llvm-dev-bounces at lists.llvm.org> on behalf of llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> > 
> > 
> > 
> >> On Feb 20, 2020, at 14:25, Johannes Doerfert <johannesdoerfert
at gmail.com <mailto:johannesdoerfert at gmail.com>> wrote:
> >> 
> >> On 02/20, Louis Dionne via llvm-dev wrote:
> >>> I know there has been significant discussion about
"moving" from
> >>> Phabricator to GitHub reviews and pull requests, etc. I'm
not
> >>> suggesting that we do anything in terms of global LLVM policy.
> >>> However, as a maintainer of libc++, I commit __a lot__ of
other
> >>> people's code for them. It would be a huge time saver for
me if I
> >>> could nicely suggest to contributors (not force them) to use
PRs
> >>> instead of Phabricator for their contributions. It would also
handle
> >>> commit attribution properly, which is a pain right now.
> >> 
> >> Don't take this as me telling you it is "actually
simple". I am
> >> interested what about the contribution is problematic? If the
libc++
> >> system doesn't have more requirements than the rest of LLVM
there might
> >> be ways to make it less painful. FWIW, here is what I do, and I
know not
> >> everyone wants to use `arc`. Ina script this could potentially
reduce
> >> the pain. Again, this is not meant to tell you it is simple or
your
> >> problems are not real.
> >> 
> >> arc patch DXXXX
> >> git pull --rebase origin master
> >> arc amend
> >> arcfilter        // see below
> >> git llvm push master
> >> 
> >> 
> >> arcfilter () { git log -1 --pretty=%B | awk
'/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0}
!p && !/^Summary:/' | git commit --amend -F - }
> > 
> >    Thanks, this indeed solves some of my problems, however not
entirely. When people submit contributions without an email address, I still
have to do some digging to find out how to attribute the change. This
shouldn't be something I even have to think about.
> > 
> >    Louis
> > 
> >> 
> >>> Would it be possible to allow GitHub PRs to be submitted on
the
> >>> monorepo so as to let individual sub-projects deal with it
however
> >>> they please? I've spoken to numerous people involved in
libc++
> >>> development and they would like to start submitting PRs (and
for the
> >>> others, we'll still accept Phabricator reviews). Perhaps
it is
> >>> possible to setup some kind of filter such that PRs touching
only
> >>> libcxx/ and libcxxabi/ can be submitted, but otherwise
they're closed
> >>> by the bot?
> >> 
> >> TBH, I feel this is yet another way of splitting the community and
in
> >> the end complicating things even more. I mean, since recently if
you
> >> want to ask a question there were the *-dev lists and the IRC. Now
we
> >> have discourse, discord on top of that with some people monitoring
only
> >> one of these and others required to monitor both. Duplicating the
way we
> >> do reviews is similarly going to require people that want to be
informed
> >> to duplicate their lookups.
> >> 
> >> Cheers,
> >> Johannes
> >> 
> > 
> >    _______________________________________________
> >    LLVM Developers mailing list
> >    llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
> >   
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=hELRqZwTPoZ26mqt3iDgkwh-f8LXjZ8HNkBIKKEysGI&s=RURnqL7Gh1L4cfsZvmuLOkD0YL9PNMBiJLJ1w0ii9Yw&e=
<https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=hELRqZwTPoZ26mqt3iDgkwh-f8LXjZ8HNkBIKKEysGI&s=RURnqL7Gh1L4cfsZvmuLOkD0YL9PNMBiJLJ1w0ii9Yw&e=>
> > 
> > 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200304/da0df41e/attachment.html>

On Wed, Mar 4, 2020 at 8:15 AM Louis Dionne via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>
> On Mar 3, 2020, at 18:48, Eric Christopher <echristo at gmail.com>
wrote:
>
> I'm one of those people ;)
>
>
> That's not something to be proud of if you expect a maintainer to
commit on your behalf. If you commit yourself, then whatever.
FWIW, I'm also one of those people. ;-) I don't think that pride needs
to factor into it -- not everyone uses arc and that's okay. I push a
lot of patches on behalf of others and have only run into one
situation where it wasn't immediately obvious who to attribute a
non-arc patch to. Asking the author for how they wanted to be
attributed was painless and sufficient.

~Aaron
>
> Louis
>
>
> -eric
>
> On Tue, Mar 3, 2020 at 2:20 PM Louis Dionne via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>>
>>
>>
>> > On Mar 3, 2020, at 17:16, Shoaib Meenai <smeenai at fb.com>
wrote:
>> >
>> > `arc patch` should preserve the author information in the original
commit, if there was any. At least it has in my experience.
>>
>> Yes, but I think people can upload raw patches to Phabricator without
using `arc diff`. I know I ran into one of these just last week where I used
Johannes' script (thanks!) and ended up still having to find the
committer's email by other means.
>>
>> Louis
>>
>> >
>> > On 3/3/20, 1:44 PM, "llvm-dev on behalf of Louis Dionne via
llvm-dev" <llvm-dev-bounces at lists.llvm.org on behalf of llvm-dev at
lists.llvm.org> wrote:
>> >
>> >
>> >
>> >> On Feb 20, 2020, at 14:25, Johannes Doerfert
<johannesdoerfert at gmail.com> wrote:
>> >>
>> >> On 02/20, Louis Dionne via llvm-dev wrote:
>> >>> I know there has been significant discussion about
"moving" from
>> >>> Phabricator to GitHub reviews and pull requests, etc.
I'm not
>> >>> suggesting that we do anything in terms of global LLVM
policy.
>> >>> However, as a maintainer of libc++, I commit __a lot__ of
other
>> >>> people's code for them. It would be a huge time saver
for me if I
>> >>> could nicely suggest to contributors (not force them) to
use PRs
>> >>> instead of Phabricator for their contributions. It would
also handle
>> >>> commit attribution properly, which is a pain right now.
>> >>
>> >> Don't take this as me telling you it is "actually
simple". I am
>> >> interested what about the contribution is problematic? If the
libc++
>> >> system doesn't have more requirements than the rest of
LLVM there might
>> >> be ways to make it less painful. FWIW, here is what I do, and
I know not
>> >> everyone wants to use `arc`. Ina script this could potentially
reduce
>> >> the pain. Again, this is not meant to tell you it is simple or
your
>> >> problems are not real.
>> >>
>> >> arc patch DXXXX
>> >> git pull --rebase origin master
>> >> arc amend
>> >> arcfilter        // see below
>> >> git llvm push master
>> >>
>> >>
>> >> arcfilter () { git log -1 --pretty=%B | awk
'/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0}
!p && !/^Summary:/' | git commit --amend -F - }
>> >
>> >    Thanks, this indeed solves some of my problems, however not
entirely. When people submit contributions without an email address, I still
have to do some digging to find out how to attribute the change. This
shouldn't be something I even have to think about.
>> >
>> >    Louis
>> >
>> >>
>> >>> Would it be possible to allow GitHub PRs to be submitted
on the
>> >>> monorepo so as to let individual sub-projects deal with it
however
>> >>> they please? I've spoken to numerous people involved
in libc++
>> >>> development and they would like to start submitting PRs
(and for the
>> >>> others, we'll still accept Phabricator reviews).
Perhaps it is
>> >>> possible to setup some kind of filter such that PRs
touching only
>> >>> libcxx/ and libcxxabi/ can be submitted, but otherwise
they're closed
>> >>> by the bot?
>> >>
>> >> TBH, I feel this is yet another way of splitting the community
and in
>> >> the end complicating things even more. I mean, since recently
if you
>> >> want to ask a question there were the *-dev lists and the IRC.
Now we
>> >> have discourse, discord on top of that with some people
monitoring only
>> >> one of these and others required to monitor both. Duplicating
the way we
>> >> do reviews is similarly going to require people that want to
be informed
>> >> to duplicate their lookups.
>> >>
>> >> Cheers,
>> >> Johannes
>> >>
>> >
>> >    _______________________________________________
>> >    LLVM Developers mailing list
>> >    llvm-dev at lists.llvm.org
>> >   
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=hELRqZwTPoZ26mqt3iDgkwh-f8LXjZ8HNkBIKKEysGI&s=RURnqL7Gh1L4cfsZvmuLOkD0YL9PNMBiJLJ1w0ii9Yw&e>>
>
>> >
>>
>> _______________________________________________
>> 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

> On Mar 4, 2020, at 08:21, Aaron Ballman <aaron at aaronballman.com>
wrote:
> 
> On Wed, Mar 4, 2020 at 8:15 AM Louis Dionne via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> 
>> On Mar 3, 2020, at 18:48, Eric Christopher <echristo at
gmail.com> wrote:
>> 
>> I'm one of those people ;)
>> 
>> 
>> That's not something to be proud of if you expect a maintainer to
commit on your behalf. If you commit yourself, then whatever.
> 
> FWIW, I'm also one of those people. ;-) I don't think that pride
needs
> to factor into it -- not everyone uses arc and that's okay. I push a
> lot of patches on behalf of others and have only run into one
> situation where it wasn't immediately obvious who to attribute a
> non-arc patch to. Asking the author for how they wanted to be
> attributed was painless and sufficient.
Having to guess what email address to use is not viable. For example, should it
be their work or their personal address? The maintainer shouldn't have to
choose that. And asking for which address to use is just a waste of time when
the contribution could have been attributed correctly in the first place by the
author. It might be okay when you commit a few patches on people's behalf,
but when you do a couple per day, it really takes its toll on productivity for
something so simply solved.

I'll just start requesting that changes be properly attributed in the first
place if I am to commit them, and that should solve this specific problem.

Louis
> 
> ~Aaron
> 
>> 
>> Louis
>> 
>> 
>> -eric
>> 
>> On Tue, Mar 3, 2020 at 2:20 PM Louis Dionne via llvm-dev <llvm-dev
at lists.llvm.org> wrote:
>>> 
>>> 
>>> 
>>>> On Mar 3, 2020, at 17:16, Shoaib Meenai <smeenai at
fb.com> wrote:
>>>> 
>>>> `arc patch` should preserve the author information in the
original commit, if there was any. At least it has in my experience.
>>> 
>>> Yes, but I think people can upload raw patches to Phabricator
without using `arc diff`. I know I ran into one of these just last week where I
used Johannes' script (thanks!) and ended up still having to find the
committer's email by other means.
>>> 
>>> Louis
>>> 
>>>> 
>>>> On 3/3/20, 1:44 PM, "llvm-dev on behalf of Louis Dionne
via llvm-dev" <llvm-dev-bounces at lists.llvm.org on behalf of llvm-dev
at lists.llvm.org> wrote:
>>>> 
>>>> 
>>>> 
>>>>> On Feb 20, 2020, at 14:25, Johannes Doerfert
<johannesdoerfert at gmail.com> wrote:
>>>>> 
>>>>> On 02/20, Louis Dionne via llvm-dev wrote:
>>>>>> I know there has been significant discussion about
"moving" from
>>>>>> Phabricator to GitHub reviews and pull requests, etc.
I'm not
>>>>>> suggesting that we do anything in terms of global LLVM
policy.
>>>>>> However, as a maintainer of libc++, I commit __a lot__
of other
>>>>>> people's code for them. It would be a huge time
saver for me if I
>>>>>> could nicely suggest to contributors (not force them)
to use PRs
>>>>>> instead of Phabricator for their contributions. It
would also handle
>>>>>> commit attribution properly, which is a pain right now.
>>>>> 
>>>>> Don't take this as me telling you it is "actually
simple". I am
>>>>> interested what about the contribution is problematic? If
the libc++
>>>>> system doesn't have more requirements than the rest of
LLVM there might
>>>>> be ways to make it less painful. FWIW, here is what I do,
and I know not
>>>>> everyone wants to use `arc`. Ina script this could
potentially reduce
>>>>> the pain. Again, this is not meant to tell you it is simple
or your
>>>>> problems are not real.
>>>>> 
>>>>> arc patch DXXXX
>>>>> git pull --rebase origin master
>>>>> arc amend
>>>>> arcfilter        // see below
>>>>> git llvm push master
>>>>> 
>>>>> 
>>>>> arcfilter () { git log -1 --pretty=%B | awk
'/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential Revision:/{p=0}
!p && !/^Summary:/' | git commit --amend -F - }
>>>> 
>>>>   Thanks, this indeed solves some of my problems, however not
entirely. When people submit contributions without an email address, I still
have to do some digging to find out how to attribute the change. This
shouldn't be something I even have to think about.
>>>> 
>>>>   Louis
>>>> 
>>>>> 
>>>>>> Would it be possible to allow GitHub PRs to be
submitted on the
>>>>>> monorepo so as to let individual sub-projects deal with
it however
>>>>>> they please? I've spoken to numerous people
involved in libc++
>>>>>> development and they would like to start submitting PRs
(and for the
>>>>>> others, we'll still accept Phabricator reviews).
Perhaps it is
>>>>>> possible to setup some kind of filter such that PRs
touching only
>>>>>> libcxx/ and libcxxabi/ can be submitted, but otherwise
they're closed
>>>>>> by the bot?
>>>>> 
>>>>> TBH, I feel this is yet another way of splitting the
community and in
>>>>> the end complicating things even more. I mean, since
recently if you
>>>>> want to ask a question there were the *-dev lists and the
IRC. Now we
>>>>> have discourse, discord on top of that with some people
monitoring only
>>>>> one of these and others required to monitor both.
Duplicating the way we
>>>>> do reviews is similarly going to require people that want
to be informed
>>>>> to duplicate their lookups.
>>>>> 
>>>>> Cheers,
>>>>> Johannes
>>>>> 
>>>> 
>>>>   _______________________________________________
>>>>   LLVM Developers mailing list
>>>>   llvm-dev at lists.llvm.org
>>>>  
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=hELRqZwTPoZ26mqt3iDgkwh-f8LXjZ8HNkBIKKEysGI&s=RURnqL7Gh1L4cfsZvmuLOkD0YL9PNMBiJLJ1w0ii9Yw&e>>>>
>>>> 
>>> 
>>> _______________________________________________
>>> 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

On Wed, Mar 4, 2020, 5:21 AM Aaron Ballman <aaron at aaronballman.com>
wrote:
> On Wed, Mar 4, 2020 at 8:15 AM Louis Dionne via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
> >
> > On Mar 3, 2020, at 18:48, Eric Christopher <echristo at
gmail.com> wrote:
> >
> > I'm one of those people ;)
> >
> >
> > That's not something to be proud of if you expect a maintainer to
commit
> on your behalf. If you commit yourself, then whatever.
>
> FWIW, I'm also one of those people. ;-) I don't think that pride
needs
> to factor into it -- not everyone uses arc and that's okay. I push a
> lot of patches on behalf of others and have only run into one
> situation where it wasn't immediately obvious who to attribute a
> non-arc patch to. Asking the author for how they wanted to be
> attributed was painless and sufficient.
>
There's no pride here for sure - I'm not even sure where you got that.
That
said I'm in complete agreement with Aaron here. It just hasn't been an
issue.


> ~Aaron
>
> >
> > Louis
> >
> >
> > -eric
> >
> > On Tue, Mar 3, 2020 at 2:20 PM Louis Dionne via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
> >>
> >>
> >>
> >> > On Mar 3, 2020, at 17:16, Shoaib Meenai <smeenai at
fb.com> wrote:
> >> >
> >> > `arc patch` should preserve the author information in the
original
> commit, if there was any. At least it has in my experience.
> >>
> >> Yes, but I think people can upload raw patches to Phabricator
without
> using `arc diff`. I know I ran into one of these just last week where I
> used Johannes' script (thanks!) and ended up still having to find the
> committer's email by other means.
> >>
> >> Louis
> >>
> >> >
> >> > On 3/3/20, 1:44 PM, "llvm-dev on behalf of Louis Dionne
via
> llvm-dev" <llvm-dev-bounces at lists.llvm.org on behalf of
> llvm-dev at lists.llvm.org> wrote:
> >> >
> >> >
> >> >
> >> >> On Feb 20, 2020, at 14:25, Johannes Doerfert <
> johannesdoerfert at gmail.com> wrote:
> >> >>
> >> >> On 02/20, Louis Dionne via llvm-dev wrote:
> >> >>> I know there has been significant discussion about
"moving" from
> >> >>> Phabricator to GitHub reviews and pull requests, etc.
I'm not
> >> >>> suggesting that we do anything in terms of global
LLVM policy.
> >> >>> However, as a maintainer of libc++, I commit __a
lot__ of other
> >> >>> people's code for them. It would be a huge time
saver for me if I
> >> >>> could nicely suggest to contributors (not force them)
to use PRs
> >> >>> instead of Phabricator for their contributions. It
would also handle
> >> >>> commit attribution properly, which is a pain right
now.
> >> >>
> >> >> Don't take this as me telling you it is
"actually simple". I am
> >> >> interested what about the contribution is problematic? If
the libc++
> >> >> system doesn't have more requirements than the rest
of LLVM there
> might
> >> >> be ways to make it less painful. FWIW, here is what I do,
and I know
> not
> >> >> everyone wants to use `arc`. Ina script this could
potentially reduce
> >> >> the pain. Again, this is not meant to tell you it is
simple or your
> >> >> problems are not real.
> >> >>
> >> >> arc patch DXXXX
> >> >> git pull --rebase origin master
> >> >> arc amend
> >> >> arcfilter        // see below
> >> >> git llvm push master
> >> >>
> >> >>
> >> >> arcfilter () { git log -1 --pretty=%B | awk
> '/Reviewers:|Subscribers:/{p=1} /Reviewed By:|Differential
Revision:/{p=0}
> !p && !/^Summary:/' | git commit --amend -F - }
> >> >
> >> >    Thanks, this indeed solves some of my problems, however
not
> entirely. When people submit contributions without an email address, I
> still have to do some digging to find out how to attribute the change. This
> shouldn't be something I even have to think about.
> >> >
> >> >    Louis
> >> >
> >> >>
> >> >>> Would it be possible to allow GitHub PRs to be
submitted on the
> >> >>> monorepo so as to let individual sub-projects deal
with it however
> >> >>> they please? I've spoken to numerous people
involved in libc++
> >> >>> development and they would like to start submitting
PRs (and for the
> >> >>> others, we'll still accept Phabricator reviews).
Perhaps it is
> >> >>> possible to setup some kind of filter such that PRs
touching only
> >> >>> libcxx/ and libcxxabi/ can be submitted, but
otherwise they're
> closed
> >> >>> by the bot?
> >> >>
> >> >> TBH, I feel this is yet another way of splitting the
community and in
> >> >> the end complicating things even more. I mean, since
recently if you
> >> >> want to ask a question there were the *-dev lists and the
IRC. Now we
> >> >> have discourse, discord on top of that with some people
monitoring
> only
> >> >> one of these and others required to monitor both.
Duplicating the
> way we
> >> >> do reviews is similarly going to require people that want
to be
> informed
> >> >> to duplicate their lookups.
> >> >>
> >> >> Cheers,
> >> >> Johannes
> >> >>
> >> >
> >> >    _______________________________________________
> >> >    LLVM Developers mailing list
> >> >    llvm-dev at lists.llvm.org
> >> >
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=hELRqZwTPoZ26mqt3iDgkwh-f8LXjZ8HNkBIKKEysGI&s=RURnqL7Gh1L4cfsZvmuLOkD0YL9PNMBiJLJ1w0ii9Yw&e>
>> >
> >> >
> >>
> >> _______________________________________________
> >> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200304/c4ccbe44/attachment.html>