Louis Dionne via llvm-dev
2020-Mar-03 21:44 UTC
[llvm-dev] Allowing PRs on GitHub for some subprojects
> 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 >
Shoaib Meenai via llvm-dev
2020-Mar-03 22:16 UTC
[llvm-dev] Allowing PRs on GitHub for some subprojects
`arc patch` should preserve the author information in the original commit, if there was any. At least it has in my experience. 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=
Louis Dionne via llvm-dev
2020-Mar-03 22:19 UTC
[llvm-dev] 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= > >
Mehdi AMINI via llvm-dev
2020-Mar-04 17:06 UTC
[llvm-dev] Allowing PRs on GitHub for some subprojects
On Tue, Mar 3, 2020 at 1:44 PM Louis Dionne via llvm-dev < 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. >I usually ask the patch author in the review how they want the attribution.> > 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://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/b4b7cd1e/attachment.html>