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

Wed Mar 4 05:42:01 PST 2020

> 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