[GRASS-dev] github: Please squash commits before merging
Panagiotis Mavrogiorgos
pmav99 at gmail.com
Tue May 28 01:06:11 PDT 2019
Hello all,
I just wanted to mention that we should try to have clean history and good
commit messages. This helps really a lot when you try to understand why
something is implemented the way it is. In this spirit, I think it is a
good idea to try to squash a Pull Request's commits before merging ,
especially if those are trivial fixes (it needs to be enabled
<https://help.github.com/en/articles/configuring-commit-squashing-for-pull-requests>
though).
For example, PR18 contained 5 commits
<https://github.com/OSGeo/grass/pull/18/commits> which were merged. The
last 4 were trivial fixes that only add noise to the history. Squashing
those would have resulted in a cleaner history and, among other things,
would have made it easier to use git bisect and git blame.
That being said, I am not arguing that the commits of each and every PR
needs to be squashed before merging. When extensive changes are being made,
it often makes sense to keep them unsquashed (e.g. to make reviewing
easier), but trivial fixes should still be squashed to the main commits.
with kind regards,
Panos
PS. BTW, when a PR only has a single commit, the "merge commit" doesn't
offer anything and it can be avoided by rebasing the feature branch. I
think it would be a good idea to try to do that too. Just to avoid any
misunderstandings, when a PR has multiple commits, the merge commit is more
or less mandatory because if you don't have it, you can't use git revert.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/grass-dev/attachments/20190528/59266e2e/attachment.html>
More information about the grass-dev
mailing list