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