<div dir="ltr">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. <div><br></div><div>What are your thoughts on that, Joel?</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jun 13, 2016 at 6:36 PM, Joel Nothman <span dir="ltr"><<a href="mailto:joel.nothman@gmail.com" target="_blank">joel.nothman@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><br></div><div>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.</div><div><br></div><div>I also find that performing the squash can be unnecessary overhead either for the merger or the PR developer.</div><div><br></div><div>I think squashing is more trouble than it's worth, except where:</div><div>* there are embarrassingly many minor commits in a PR</div><div>* squashing first makes a rebase easier because of concurrent changes to the codebase</div><div>* otherwise for cosmetic reasons only when there is low reviewing activity on the PR</div><div><br></div><div>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.</div><div><br></div><div>Do others agree that a culture of amending commits in the ordinary case is counterproductive?</div><div><br></div><div>(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.)</div></div>
<br>_______________________________________________<br>
scikit-learn mailing list<br>
<a href="mailto:scikit-learn@python.org">scikit-learn@python.org</a><br>
<a href="https://mail.python.org/mailman/listinfo/scikit-learn" rel="noreferrer" target="_blank">https://mail.python.org/mailman/listinfo/scikit-learn</a><br>
<br></blockquote></div><br></div>