From our
SoftServ restrospective we want to look at our merge strategy as we move to Github.After reviewing the goals of our code repository and merge strategies of Github, my recommendation is to adopt and enforce Github’s default Merge pull request: Create a Merge Commit method.
There are three goals:
Reading About merge methods on GitHub we have:
main
branch.main
branch.main
branch.In the About merge methods on GitHub there are several diagrams that help visualize the impact on the commit graph; I recommend reviewing those diagrams.
Method | Ease of Reverting a PR | Preserve Commit Message | Write Clean History |
---|---|---|---|
Merge | Add a Revert Commit for Merge Commit | All messages preserved | Multi-line interweaving history |
Squash | Revert the Squashed Commit for the PR | Messages are obliterated | Single-line History |
Rebase | Need to know the SHAs part of PR | All messages are preserved | Single-line History |
In other words, each of the above methods is weak on one of the goals.
For the Squashing your merge commits Github does append to the PR’s title the PR number. This allows you to associate this change with the full context of the Pull Request. Note: This functionality creates a vendor lock-in for viewing aspects of your code’s history.
For Rebasing and merging your commits to compensate for it’s weakness, namely reverting a change, we would need to look at the single-line history and determine where to revert. This exercise would mean reading the subjects, looking at recently merged PRs, and comparing the SHA of the previous deploy. In other words, notable documentation and guidance to help folks rollback a change during a potential outage.
Further, in Rebasing and merging your commits you really need strict discipline about atomic commits; because if they are not atomic, and you pick the wrong point to rollback you could be creating even further mayhem.
For Merge pull request the multi-line interweaving of history creates visual chatter for orienting to the history of the code-base.
I posit that a “clean history” is the lowest priority goal. Why? Because losing our commit messages loses prior context; context which might help current and future developers. And we want to avoid introducing additional lookup steps to rollback a code change.
Given that we are migrating from Gitlab to Github, we have precedence to be cautious of reliance on vendor features. More importantly, we must recognize that due to our past merge strategies, we are leaving behind some of the information encoded in past commit messages. Thus I think this should remove the Squashing your merge commits from the viable options.
Given that Rebasing and merging your commits can result in additional cognitive load during an outage, I want to weigh that cognitive load versus the “downside” of Merge pull request; namely interweaving history.
To perform a rollback on a Merge pull request method requires that we pick the merge requests we want to rollback and then perform a revert on those requests. If our rollback strategy does not involve reverting commits on main, we need to consider how to find the correct SHA for deploying our application.
Multi-line interweaving history makes that harder; you need to read the commit graph or bring different strategies to orient to the code.
One strategy for finding the commit is to use git log --merges
to show the merge commits that introduced each pull request. Find the problematic SHA merge commit and pick the merge commit prior.
Further, with git log --merges
we can see the Single-line “conceptual” history of changes. We rely on the pull request title for information regarding all of the commits that comprised that merge request.
Looking at Hyrax, they leave the decision of merge method to the person who performs the merge.
At Forem, they apply the Squash your merge commits method; I found myself regularly and often jumping to the PR in Github to see the commit messages and individual changes.
As stated in the Executive Summary and following on my discussion, I recommend that we adopt and enforce by configuration the Merge pull request: Create a merge request method.