[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.
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...
More information about the llvm-dev