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

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

Ah, that's a fair point. Yeah, that's unfortunate.

On 3/3/20, 2:21 PM, "ldionne at apple.com on behalf of Louis Dionne"
<ldionne at apple.com> 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=
    > 
    >

I'm one of those people ;)

-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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200303/b2bc67b4/attachment.html>

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