W3C home > Mailing lists > Public > public-html-media@w3.org > November 2015

Re: Please squash all commits before merging pull requests

From: Mark Watson <watsonm@netflix.com>
Date: Thu, 5 Nov 2015 11:47:05 -0800
Message-ID: <CAEnTvdA5yC6r=ac78ww=M+C7orA=T6wV0M1dtSnqLcKbXvfQDw@mail.gmail.com>
To: David Dorwin <ddorwin@google.com>
Cc: "public-html-media@w3.org" <public-html-media@w3.org>, Matt Wolenetz <wolenetz@google.com>, "Jerry Smith (WINDOWS)" <jdsmith@microsoft.com>
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
>>>>
>>>>
>>>
>>
>
Received on Thursday, 5 November 2015 19:47:37 UTC

This archive was generated by hypermail 2.3.1 : Thursday, 5 November 2015 19:47:37 UTC