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

Re: Please squash all commits before merging pull requests

From: David Dorwin <ddorwin@google.com>
Date: Thu, 5 Nov 2015 11:59:58 -0800
Message-ID: <CAHD2rshXEBJ3dRRdgz+w3j+QMVDKxBHKokmP05qEY3UuJ08NSw@mail.gmail.com>
To: Mark Watson <watsonm@netflix.com>
Cc: "public-html-media@w3.org" <public-html-media@w3.org>, Matt Wolenetz <wolenetz@google.com>, "Jerry Smith (WINDOWS)" <jdsmith@microsoft.com>
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.

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
>>>>>
>>>>>
>>>>
>>>
>>
>
Received on Thursday, 5 November 2015 20:00:46 UTC

This archive was generated by hypermail 2.3.1 : Thursday, 5 November 2015 20:00:47 UTC