[llvm-dev] Allowing PRs on GitHub for some subprojects

Louis Dionne via llvm-dev llvm-dev at lists.llvm.org
Wed Mar 4 07:50:02 PST 2020



> On Mar 4, 2020, at 09:55, Eric Christopher <echristo at gmail.com> wrote:
> 
> 
> 
> On Wed, Mar 4, 2020, 5:21 AM Aaron Ballman <aaron at aaronballman.com <mailto: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 <mailto:llvm-dev at lists.llvm.org>> wrote:
> >
> > On Mar 3, 2020, at 18:48, Eric Christopher <echristo at gmail.com <mailto: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. 

As discussed on IRC, it turns out the Developer policy actually has a section on that for new committers [1]:

	Prior to obtaining commit access, it is common practice to request that someone with commit access commits on your behalf. When doing so, please provide the name and email address you would like to use in the Author property of the commit.

So, I'll just point contributors to that sentence when attribution isn't obvious.

Louis

[1]: https://llvm.org/docs/DeveloperPolicy.html#new-contributors

> 
> 
> 
> ~Aaron
> 
> >
> > 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>
> >
> >
> > _______________________________________________
> > 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/66955d29/attachment-0001.html>


More information about the llvm-dev mailing list