[llvm-dev] Enable Contributions Through Pull-request For LLVM
Daniel Sanders via llvm-dev
llvm-dev at lists.llvm.org
Fri Nov 8 10:44:25 PST 2019
> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-dev