[scikit-learn] The culture of commit squashing

[Andy]

I agree that it adds some annoying overhead.
For me, one of the main motivations is to make cherry picks for bugfix 
releases easier.
It's very hard to cherry pick things that are spread out over many 
commits, and it's hard to find the relevant bug fixes among hundreds of 
minor commits.
This really mostly impacts me an Olivier and maybe Gael (not sure if you 
did that at some point, too).

It is somewhat related to our practice of merging by rebase, which I 
think we mostly stopped.
When merging by rebase, you don't have merge commits, so finding out 
which commit belongs to which changeset is hard.
I think the rebasing is actually not a great idea for that reason (and 
also it breaks the nice links from commits to PRs which github provides 
these days).

If we do standard merges and don't squash, I guess the cherry picking is 
still possible, though somewhat harder.
It's not that common a use-case, though, and I guess we can remove the 
hassle of the rebase if it's too much of a nuisance.



On 06/13/2016 09:36 PM, Joel Nothman 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/342195b4/attachment-0001.html>