| Re: [rdf4j-dev] Squashing before merging |
|
Hi Jeen, One minor remark perhaps, the
https://rdf4j.org/documentation/developer/
workflow still mentions the use of “issues/” branches in PRs So I guess there should be a small note on using “GH-issue-number” as the commit message
(or perhaps also using GH-issue number for naming the branches themselves ? otherwise it could be a bit confusing/cumbersome to use issues/ in branch name and GH in commit message ?) Best regards Bart From: rdf4j-dev-bounces@xxxxxxxxxxx <rdf4j-dev-bounces@xxxxxxxxxxx>
On Behalf Of Jeen Broekstra We've been using Github squash and merge for a little while now and I must admit I still don't like the downsides described below. I am also conscious of the fact that in keeping our develop and master branch in sync, it is all too easy
to make a mistake and use squash and merge. More generally: cleaning up a branch's commit history should be the responsibility of the branch author, not of whoever happens to do the merge.
I propose that we stop using squash and merge (I'll physically disable it), and instead each PR author makes sure to squash their feature branch locally where appropriate before merging the PR, through either a fast-forward (using github
rebase) or (if that is not possible) a normal merge commit. It's a little more work, but it's a decent compromise between keeping the history clean/readable and still having a good workflow as well. If we adopt this, I'll write a little blurb in the contributor guide about how to do squashing so we can point contributors to this.
A related request: before merging a PR, please make sure that both the PR title and each commit message start with the issue number: "GH-1234 my feature". The nice thing about this is that when browsing the code in Github, the issue number
in the commit message clickable, which is incredibly useful when digging through when/why a particular change was made.
Squashing gives you the option not only to create fewer commits on your branch, but also to rewrite your commit messages - so I'm perfectly fine with "wip" as a commit message while you're working on something, as long as you clean it up
when it's ready to be merged. Let me know if you have any questions or concerns. If I don't hear anything, I'll assume everyone is ok with this and will disable squash and merge on the repo. Cheers, Jeen |