[scikit-learn] The culture of commit squashing

Jacob Schreiber jmschreiber91 at gmail.com
Mon Jun 13 22:40:47 EDT 2016


My research work involves frequently contributing small changes. I like to
keep these around as a record of what I've done, until I've finished with
that part of the code. However, I also hate having large numbers of commits
(frequently can commit 50+ times a day without much substantitve progress).
To combine these, usually I will avoid squashing commits in a branch until
right before I merge it. This way you can review everything which has been
done until you're finished with that branch, but also avoid having a large
number of trivial commits. In this case, only after you've been given MRG
+2 would you squash the PR. That would have a negative side effect of
preventing the second reviewer from quickly merging the branch, though.

What are your thoughts on that, Joel?

On Mon, Jun 13, 2016 at 6:36 PM, Joel Nothman <joel.nothman at gmail.com>
wrote:

> 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.)
>
> _______________________________________________
> scikit-learn mailing list
> scikit-learn at python.org
> https://mail.python.org/mailman/listinfo/scikit-learn
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scikit-learn/attachments/20160613/0d1b0e98/attachment.html>


More information about the scikit-learn mailing list