llvm dev - Nov 2019 - Enable Contributions Through Pull-request For LLVM

> On Nov 7, 2019, at 5:54 PM, Daniel Sanders via llvm-dev <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.

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.

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191107/42b90ae8/attachment.html>

> On Nov 7, 2019, at 22:31, Chris Lattner <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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20191108/361ad5d8/attachment.html>

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>