Re: Please squash all commits before merging pull requests

On Thu, Nov 5, 2015 at 11:59 AM, David Dorwin <ddorwin@google.com> wrote:

> Would squashing the branch before rebasing work? That's generally the best
> way (as far as I know) to avoid such issues, but I haven't tried it with
> merges.
>

​I thought that too, but when you look at the log in the branch, the
commits that are unique to the branch are spread all through the history,
interspersed with the main branch commits you have just merged.

As it turned out with PR#87 the conflicts were minor and in several cases
only on index.html.



>
> On Thu, Nov 5, 2015 at 11:47 AM, Mark Watson <watsonm@netflix.com> wrote:
>
>> I find that when following this procedure with a PR / branch which has
>> been kept up-to-date with other changes on gh-pages (via merges into the
>> branch), then the rebase step encounters many conflicts.
>>
>> I think the problem is that rebasing attempts to replay the changes in
>> the branch on the head of gh-pages. The older commits in the branch contain
>> changes against an older version of gh-pages and these do not replay
>> cleanly.
>>
>> Is this expected ? Any ideas how to avoid this ?
>>
>> ...Mark
>>
>> On Mon, Nov 2, 2015 at 12:04 PM, David Dorwin <ddorwin@google.com> wrote:
>>
>>> This is now documented in more detail at
>>> https://github.com/w3c/encrypted-media/blob/gh-pages/TEAM.md
>>>
>>> On Wed, Sep 23, 2015 at 11:52 AM, David Dorwin <ddorwin@google.com>
>>> wrote:
>>>
>>>> I just tested this proposal on a couple of my pull requests. It's
>>>> slightly more work, but it seemed to work well.
>>>>
>>>> The scripts from https://github.com/whatwg/html/blob/master/TEAM.md
>>>> need to be updated to replace "master" with "gh-pages" as below. After
>>>> running "pr <pr#> and pushing, I copied the commit SHA, wrote a comment
>>>> that said "Merged as <SHA>." and clicked the "Close pull request" button to
>>>> post that comment.
>>>>
>>>>
>>>> pr () {
>>>>   git fetch origin refs/pull/$1/head:refs/remotes/origin/pr/$1 --force
>>>>   git checkout -b pr/$1 origin/pr/$1
>>>>   git rebase gh-pages
>>>>   git checkout gh-pages
>>>>   git merge pr/$1 --ff-only
>>>> }
>>>>
>>>> mypr () {
>>>>   git checkout $1
>>>>   git rebase gh-pages
>>>>   git push origin $1 --force
>>>>   git checkout gh-pages
>>>>   git merge $1 --ff-only
>>>> }
>>>>
>>>> On Fri, Sep 11, 2015 at 11:50 AM, David Dorwin <ddorwin@google.com>
>>>> wrote:
>>>>
>>>>> *New proposal:* Use the same process being followed for the HTML spec
>>>>> <https://github.com/whatwg/html> as documented at
>>>>> https://github.com/whatwg/html/blob/master/TEAM.md.
>>>>>
>>>>> Most importantly, "The green button shall not be pushed. Each change
>>>>> needs to result in a single commit on the master branch, with no merge
>>>>> commits." The page provides some scripts that are useful for manually
>>>>> merging pull requests. These appear to preserve the original author while
>>>>> adding a single commit as can be seen at
>>>>> https://github.com/whatwg/html/commits/master.
>>>>>
>>>>> For pull requests with multiple commits, squashing with git rebase -i
>>>>> may still be required after running the pr script.
>>>>>
>>>>>
>>>>> *Background/Motivation*
>>>>>
>>>>> After following the previous proposal, Mark wrote
>>>>> <https://github.com/w3c/encrypted-media/issues/86#issuecomment-139569822>
>>>>> :
>>>>>
>>>>>> Hmm, whilst there is only one commit in the PR, I see two in the main
>>>>>> repository now it is merged - the commit from the PR and the merge. Is that
>>>>>> correct ?
>>>>>>
>>>>>
>>>>> He's referring to this:
>>>>>
>>>>>    1. [image: @mwatson2] <https://github.com/mwatson2>
>>>>>
>>>>>    Merge pull request
>>>>>    <https://github.com/w3c/encrypted-media/commit/49b98d2d77a63b1d3bb99d8a122b5370f709f870>
>>>>>     #90 <https://github.com/w3c/encrypted-media/pull/90> from
>>>>>    mwatson2/issue-86
>>>>>    <https://github.com/w3c/encrypted-media/commit/49b98d2d77a63b1d3bb99d8a122b5370f709f870>
>>>>>     … <https://github.com/w3c/encrypted-media/commits/gh-pages#>
>>>>>    mwatson2
>>>>>    <https://github.com/w3c/encrypted-media/commits/gh-pages?author=mwatson2> authored 3
>>>>>    hours ago
>>>>>    49b98d2
>>>>>    <https://github.com/w3c/encrypted-media/commit/49b98d2d77a63b1d3bb99d8a122b5370f709f870>
>>>>>
>>>>>    <https://github.com/w3c/encrypted-media/tree/49b98d2d77a63b1d3bb99d8a122b5370f709f870>
>>>>>    2. [image: @mwatson2] <https://github.com/mwatson2>
>>>>>
>>>>>    Fix
>>>>>    <https://github.com/w3c/encrypted-media/commit/e5c8d2c8a2db32b65f46c49442eb4c31dff69bbd>
>>>>>     #86 <https://github.com/w3c/encrypted-media/issues/86>: Rename
>>>>>    'tracked' session type to 'persistent-usage-record'
>>>>>    <https://github.com/w3c/encrypted-media/commit/e5c8d2c8a2db32b65f46c49442eb4c31dff69bbd>
>>>>>    mwatson2
>>>>>    <https://github.com/w3c/encrypted-media/commits/gh-pages?author=mwatson2> authored 8
>>>>>    days ago
>>>>>    e5c8d2c
>>>>>    <https://github.com/w3c/encrypted-media/commit/e5c8d2c8a2db32b65f46c49442eb4c31dff69bbd>
>>>>>
>>>>>    <https://github.com/w3c/encrypted-media/tree/e5c8d2c8a2db32b65f46c49442eb4c31dff69bbd>
>>>>>
>>>>> This is how GitHub and the “Merge pull request” button work. Two
>>>>> consecutive commits is better than an unsquashed disjoint history, but it
>>>>> still adds clutter. Many projects believe this is ugly and thus avoid using
>>>>> the button.
>>>>>
>>>>> Other policies and solutions:
>>>>>
>>>>>    -
>>>>>    http://blog.spreedly.com/2014/06/24/merge-pull-request-considered-harmful/
>>>>>    -
>>>>>    http://django.readthedocs.org/en/latest/internals/contributing/committing-code.html
>>>>>
>>>>>
>>>>> On Fri, Sep 4, 2015 at 11:13 AM, David Dorwin <ddorwin@google.com>
>>>>> wrote:
>>>>>
>>>>>> GitHub pull requests are a great tool, but they can also complicate
>>>>>> the commit log. What once was a relatively linear commit history can become
>>>>>> a mess of parallel "branches" over long periods of time. This is most
>>>>>> obvious in tools like gitk, but the GitHub history is also confusing
>>>>>> because the commits from a single pull request may be scattered throughout
>>>>>> the commit log.
>>>>>>
>>>>>> This is most important for merges from upstream into the pull request
>>>>>> as the author keeps the branch up-to-date. These merges appear as commits
>>>>>> in the history once the pull request is merged. However, updates based on
>>>>>> review feedback or fixing spelling also appear as separate commits, which
>>>>>> can make it hard to see exactly what was committed.
>>>>>>
>>>>>> See http://programmers.stackexchange.com/a/263172 for additional
>>>>>> explanation.
>>>>>>
>>>>>>
>>>>>> I propose:
>>>>>>
>>>>>> To minimize the impact, please squash all commits in a pull request
>>>>>> into a single commit before merging it. Committers (editors) should make
>>>>>> sure commits have been squashed before merging their own or others' pull
>>>>>> requests.
>>>>>>
>>>>>> There will be exceptions. For example, sometimes a pull request might
>>>>>> have multiple distinct actions (i.e. do something then rename a variable),
>>>>>> in which case the branch might be squashed into two commits.
>>>>>>
>>>>>>
>>>>>> Details:
>>>>>> The squashing happens in the branch and updates the pull request
>>>>>> before it is merged. Thus, you should be able to view the results in GitHub
>>>>>> before merging into the mainline. Most of the magic happens with "git
>>>>>> rebase -i <base-commit>" on your local repository. Be sure to pick the
>>>>>> right base-commit for <base-commit>. (For merges, this appears to be the
>>>>>> master branch.) Then, you need to (force) push your changes to GitHub.
>>>>>>
>>>>>> The one drawback is that the previous commits are wiped from the
>>>>>> commit history for your branch. That means the review history and comments
>>>>>> are no longer browsable (as far as I can tell). They appear to still be
>>>>>> available if you have the URLs, though. Thus, if you want to maintain
>>>>>> history, it might make sense to create a new branch and/or pull request
>>>>>> with the squashed commit.
>>>>>>
>>>>>> Note: You can also fix commit messages using the "reword" feature of
>>>>>> "git rebase -i". For example, if you forgot to refer to the issue number.
>>>>>>
>>>>>> References:
>>>>>>
>>>>>>    - http://programmers.stackexchange.com/a/263172
>>>>>>    -
>>>>>>    https://github.com/ginatrapani/todo.txt-android/wiki/Squash-All-Commits-Related-to-a-Single-Issue-into-a-Single-Commit
>>>>>>    -
>>>>>>    https://github.com/edx/edx-platform/wiki/How-to-Rebase-a-Pull-Request
>>>>>>    -
>>>>>>    http://eli.thegreenplace.net/2014/02/19/squashing-github-pull-requests-into-a-single-commit
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>