Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Consider disabling merge commits, requiring squash (and maybe rebase) only #196

Open
agoode opened this Issue Mar 8, 2017 · 2 comments

Comments

Projects
None yet
2 participants
Owner

agoode commented Mar 8, 2017

https://github.com/openslide/openslide/settings

Merge commits add a new commit without content and can clutter history. Squash commits convert an entire pull request into a single commit, keeping minor changes made during review time out of the committed history.

It is a personal preference how this is configured, but I have found squash commits to work better than other styles when using code review.

Rebase commits are a little better than merge commits, but still can generate extra commit history.

agoode added the enhancement label Mar 8, 2017

agoode added this to the Robust code review process milestone Mar 8, 2017

bgilbert removed this from the Robust code review process milestone Mar 8, 2017

Owner

bgilbert commented Mar 8, 2017

The advantage to merge commits is that they preserve the development history. When a change evolves due to review comments, I'd generally prefer to have that history maintained in the repo, rather than obscuring the reason that things were done a particular way. The exception is very messy PRs, in which case some sort of history rewriting may make sense. My approach at the moment is to ask submitters to rewrite and force-push when needed for e.g. formatting changes, and otherwise to preserve history.

Rebase commits preserve messy history, for good and for ill, but obscure the fact that the changes were developed and tested against a different base commit than the one they were merged against. This seems suboptimal.

Owner

agoode commented Mar 8, 2017

The other benefit of squash commits is that it makes bisecting easier, both automatically and manually.

I am ok with either way, but when we decide, we should update/rewrite CONTRIBUTING.txt (as .md maybe) and document the process there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment