[scikit-learn] The culture of commit squashing

[Matthieu Brucher]

I don't even think that squashing them before the merge is actually sound.
You will still need the history of why something happened several years
down the road (and rebasing actually has a similar issue). This bit me
quite often (having just one big commit to analyze after a merge from
ancient VCS). Git was created to keep the history when merging, why would
we explicitly remove knowledge?
Just my 2 cents.

2016-06-14 4:40 GMT+02:00 Jacob Schreiber <jmschreiber91 at gmail.com>:

> 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
>>
>>
>
> _______________________________________________
> scikit-learn mailing list
> scikit-learn at python.org
> https://mail.python.org/mailman/listinfo/scikit-learn
>
>


-- 
Information System Engineer, Ph.D.
Blog: http://blog.audio-tk.com/
LinkedIn: http://www.linkedin.com/in/matthieubrucher
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scikit-learn/attachments/20160614/777f42d8/attachment.html>