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: Mon, 9 Nov 2015 15:25:46 -0800
Message-ID: <CAHD2rshCxDkiAUWgxhuzyqwci=oRqesDmGWg3EHLsnbzw4zULg@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>
For future reference:

Using -p with git rebase preserves the merges:
http://stackoverflow.com/questions/4783599/rebasing-a-git-merge-commit

You might still have to resolve the same conflicts. Git rerere helps with
this: https://git-scm.com/blog/2010/03/08/rerere.html

If you didn't have rerere enabled before the merge, see
http://apasca.blogspot.com/2012/02/git-rebasing-merge-commits.html.

On Thu, Nov 5, 2015 at 12:22 PM, Mark Watson <watsonm@netflix.com> wrote:

>
>
> 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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Received on Monday, 9 November 2015 23:26:35 UTC

This archive was generated by hypermail 2.3.1 : Monday, 9 November 2015 23:26:36 UTC