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, 2 Nov 2015 12:04:30 -0800
Message-ID: <CAHD2rsiSd+UEzVx4_KND6mOseUZvbU+RgzXEWBx5eJKMEstOeA@mail.gmail.com>
To: "public-html-media@w3.org" <public-html-media@w3.org>, Matt Wolenetz <wolenetz@google.com>, Mark Watson <watsonm@netflix.com>, "Jerry Smith (WINDOWS)" <jdsmith@microsoft.com>
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, 2 November 2015 20:05:23 UTC

This archive was generated by hypermail 2.3.1 : Monday, 2 November 2015 20:05:24 UTC