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

Daniel Sanders via llvm-dev llvm-dev at lists.llvm.org
Fri Nov 8 11:24:25 PST 2019



> On Nov 8, 2019, at 08:28, David Greene <greened at obbligato.org> wrote:
> 
> Daniel Sanders via llvm-dev <llvm-dev at lists.llvm.org> writes:
> 
>> Personally, I'd like us to drop the linear history requirement but I
>> know there's some strong opposition there. The main reason I'd like us
>> to accept non-linear history in conjunction with pull requests is that
>> it means that the commits that were tested (whether by the author or
>> by CI) still exist in the repository after the merge. If we
>> rebase+push like we do today or squash+merge as in this proposal, then
>> the information that a particular commit passed testing (or failed a
>> particular test) gets lost.
> 
> -1 from me.  We have one project where we decided to "keep the original
> commits" as you say above and another where we enforce linear history.
> The linear history project is *much* easier to follow.

FWIW, I can't say I've had any real difficulty following non-linear history. I think that's mainly because even non-linear history has a linear history in it (the first-parent one that that article is trying to protect). Then in addition to the linear first-parent history you have more detailed linear histories on the side if you want to see them. It's certainly possible to get into a mess if your pull requests contain merge commits though.

> With merge
> commits you start running into issues like foxtrot commits
> (https://blog.developer.atlassian.com/stop-foxtrots-now) and the whole
> thing easily becomes a mess.  There may be various rules/plugins/etc.
> that attempt to enforce "good" behavior but in the long run my
> experience has been that simpler rules make things simpler.
> 
>                     -David

The rule you need to avoid those kinds of mess is essentially the same one as the one we currently have for pushing directly to master: No merge commits in the pull request.
Or to put it another way that accounts for push to master and pull requests: Humans aren't allowed to make merge commits.

From that article:
> You could disable direct pushes to master, and hope that pull-requests never merge with fast-forward. 

That's actually how our downstream repo works except we don't rely on hope. The server is the one merging pull requests and it's doing the merge with --no-ff. It's the equivalent of the 'Create merge commit' option in GitHub.

Based on reading GitHub's documentation (https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/merging-a-pull-request#merging-a-pull-request-on-github <https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/merging-a-pull-request#merging-a-pull-request-on-github>) all three of the options for merging pull requests will preserve a good first-parent linear history. GitHub doesn't ban pushing to master of course and we shouldn't do that as it will make pull-requests mandatory.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191108/4ddbfca5/attachment.html>


More information about the llvm-dev mailing list