[llvm-dev] Enable Contributions Through Pull-request For LLVM

Fri Nov 8 11:14:30 PST 2019

Very broadly my $0.02.

At the Women in Compilers event before this year's dev meeting one of the things repeatedly cited as a keeping new contributors away was that LLVM doesn't use modern or familiar tools. GitHub (love it or hate it) is the place most people look for open source project hosting, and it provides the workflows people are familiar with. I believe we should strive to fit that mold as much as possible (within common sense and good practice) to make developing LLVM resemble patterns that other projects use.

I also feel that it is worth noting GitHub provides a command line interface that can be used to create pull requests, so there is almost no workflow difference to a command line focused user (see: https://github.com/github/hub <https://github.com/github/hub>).

I don't know what percentage of us use arcanist vs uploading via the website, but the officially supported GitHub command line and UI tools are pretty great.

Lastly, something people often overlook is that you can actually edit files and create pull requests entirely on GitHub's web interface. This is a huge benefit for people making documentation changes, code comment changes, and other extremely simple changes. The fact that people can edit docs on my phone or tablet without needing to even clone the repository will be a huge reduction in barrier for documentation changes.

If you can't tell, I'm strongly in favor of enabling pull requests.

-Chris

> On Nov 8, 2019, at 10:44 AM, Daniel Sanders via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> 
> 
>> On Nov 7, 2019, at 22:31, Chris Lattner <clattner at nondot.org <mailto:clattner at nondot.org>> wrote:
>> 
>> 
>> 
>>> On Nov 7, 2019, at 5:54 PM, Daniel Sanders via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>> 
>>> 
>>> -1 to "squash and merge" being the only option if rebase+push (--ff-only) is possible. I'd rather we use our judgement to decide what's appropriate for the pull request at hand rather than have a blanket rule.
>>> 
>>> Personally, if I have multiple commits (typically 2-5) in a pull request it's because there's incremental steps that I'd like to preserve. For example:
>>> 1. Add assembler support for X
>>> 2. Add codegen support for X
>>> 
>>> 1. Move X to libY. NFC
>>> 2. Add support for Z
>>> We could do each step in individual pull requests but it's unnecessary overhead.
>> 
>> I agree that pull requests add additional overhead, but they can be scripted to reduce it.  My primary goal is to achieve these things set out in the developer policy:
>> http://llvm.org/docs/DeveloperPolicy.html#incremental-development <http://llvm.org/docs/DeveloperPolicy.html#incremental-development>
>> 
>> By making each of these steps explicit PR’s, they get individual testing (when we have integrated presubmit testing, which we’ll hopefully get to), code reviewers are reviewing each atomic step (and not the result of a bunch of walk), etc.
> 
> A lot of setups do only test the tip of the PR but there's nothing really stopping a CI system from testing each commit. FWIW, we don't test the individual commits in our downstream repo but that's really only because we have very extensive pre-merge testing and we'd need many more bots to cope with the increased load.
> 
>> In my opinion, bundling up many small patches into a single large PR subverts a lot of the goals of incremental development.  It is better to propose the small patches as individual patches.
> 
> I completely agree with the first bit of that. I think we only disagree on the measurement there with me measuring it in in a more subjective/flexible manner than the number of commits.
> 
>> 
>> -Chris
> 
> _______________________________________________
> 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/20191108/3511cef2/attachment.html>