Re: Please squash all commits before merging pull requests

*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 Friday, 11 September 2015 18:50:51 UTC