[scikit-learn] The culture of commit squashing

Joel Nothman joel.nothman at gmail.com
Mon Jun 13 21:36:52 EDT 2016


For the last few years, there's been a notion that we should squash PRs
down to a single commit before merging. Squashing can give a cleaner commit
history, and avoid overrepresentation of minor work given silly commit
count metrics used by Github and others. I'm not sure if there are other
motivations.

Recently I've seen several contributors amending commits and
force-pushing changes. I find this disruptive to the reviewing process in a
number of ways (links are broken; what's changed is hard to discern, when
it could have been indicated in a commit message and diff; etc.). I have
had to ask these several users to cease and desist.

I also find that performing the squash can be unnecessary overhead either
for the merger or the PR developer.

I think squashing is more trouble than it's worth, except where:
* there are embarrassingly many minor commits in a PR
* squashing first makes a rebase easier because of concurrent changes to
the codebase
* otherwise for cosmetic reasons only when there is low reviewing activity
on the PR

While squashing is far from the slowest part of our review process, being
able to hit the merge button and move on would be great.

Do others agree that a culture of amending commits in the ordinary case is
counterproductive?

(And apologies for wasting your time on such a silly issue, but I'm sick of
clicking links in emails to find the commit's disappeared.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scikit-learn/attachments/20160614/04b97bdb/attachment.html>


More information about the scikit-learn mailing list