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

Wed Mar 4 06:55:28 PST 2020

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>