github: Please squash commits before merging

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

github: Please squash commits before merging

Panagiotis Mavrogiorgos
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 though).

For example, PR18 contained 5 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.


_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: github: Please squash commits before merging

Martin Landa
Hi,

please extend https://trac.osgeo.org/grass/wiki/HowToGit#Mergingofpullrequests

Thanks! Ma

út 28. 5. 2019 v 10:06 odesílatel Panagiotis Mavrogiorgos
<[hidden email]> napsal:

>
> 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 though).
>
> For example, PR18 contained 5 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.
>
> _______________________________________________
> grass-dev mailing list
> [hidden email]
> https://lists.osgeo.org/mailman/listinfo/grass-dev



--
Martin Landa
http://geo.fsv.cvut.cz/gwiki/Landa
http://gismentors.cz/mentors/landa
_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|

Re: github: Please squash commits before merging

Markus Neteler
Hi,

On Tue, May 28, 2019 at 1:23 PM Martin Landa <[hidden email]> wrote:
> please extend https://trac.osgeo.org/grass/wiki/HowToGit#Mergingofpullrequests

done (thanks Panos!):
https://trac.osgeo.org/grass/wiki/HowToGit#MergingofPullRequests

Please feel free to improve the text.

best
Markus
_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev