đŸ Archived View for dcreager.net âș 2018 âș 04 âș git-workflows.gmi captured on 2024-08-25 at 00:20:46. Gemini links have been rewritten to link to archived content
âŹ ïž Previous capture (2024-05-26)
-=-=-=-=-=-=-
2018-04-19 (last updated 2018-05-02)
NOTE: Though I'm currently employed by GitHub, I wrote this article before I started working there! This post (like all of the posts on this site) contains my own personal views, and not those of my employer.
A couple of recent tweets really resonated me.
Aditya started an interesting conversation:
If youâd asked me a few weeks ago whether I preferred squash-merges or merge commits, Iâd have been somewhat ambivalent.
After today, Iâm pretty sold on squash-merge as the better strategy. Itâs not without drawbacks, but on net, itâs still so much better to work with.
âAditya Mukerjee (@chimeracoder) April 17, 2018
I've always really liked having a âcleanâ history in the git logs of my projects. They look nicer! There's a real joy to running âgit logâ (or even better, âgit log --graph --allâ!) on a project that's maintained a beautiful log. There are a number of tangible benefits, too, which others have described well enough that I don't need to repeat them here.
How to write a git commit message [Chris Beams]
Version control best practices [git tower]
Commit messages: Good practices [Tarin Gamberini]
More than that, though, for a long time I assumed that if you were using GitHub pull requests, there was One True Way to produce a clean history, echoed by JesĂșs:
Best merges to me are the fast-forward merges that bring a number of meaninful, atomic, functional and self-contained commits! Not always possible...
âJesĂșs GarcĂa Crespo (@sevein) April 18, 2018
But recently, just like Aditya, I've had a change of heart! I realized that I was thinking about pull requests, and what they represent in terms of the final commit log I'm aiming for, in the wrong way. In this post I'll describe the come-to-Aditya moment that made me appreciate squash-commits. And along the way I'll have described two workflows that make it easy to keep a clean history: one that doesn't use GitHub at all (and mimics how we did git-driven development before GitHub existed), and one that does.
Before we can figure out how to achieve a clean history, we have to decide what exactly we mean by one. If you have a âclean historyâ, then if I look at the log of git commits for the project, all of the following should be true:
This is sometimes stated as âyour commit must make exactly one changeâ, or âyour commit should be as small as possible, but no smallerâ.
What does this mean? You should be able to describe â very briefly and precisely â what _single_ change the commit makes to the code base. What can the code do now that it couldn't do before? What bug was there before that isn't now?
If your commit makes two or more changes to the code, each of which you could describe very briefly, then you should split that into two or more commits! If your commit doesn't contain _all_ of the modifications needed for that change, then it's not âself-containedâ, and needs to be bigger!
This is a corollary to rule #1: if the test suite doesn't pass, then you've either added a new test without adding the new code that it's meant to cover, or you've updated some code without updating all of the effected test cases. In both cases, your commit isn't really self-contained.
Tim's post (now >10 years old!) contains the clearest description of the Generally Accepted Rules for writing commit messages. These rules are pretty strict! You can probably loosen some of grammar recommendations if you like. (...not really, just train yourself to follow all of them!)
But you really should keep to a short 50-character subject line + longer description. And that description should actually be descriptive! Describe what the commit is trying to accomplish, and why. You might end up copying a lot of text from your issue tracker, or from comments or documentation that is also in the code itself. And that's okay! I want to be able to see at least a high-level overview of each change without needing anything other than âgit logâ.
A note about git commit messages [Tim Pope]
This rule is probably the least obvious. My hunch is that if you canvas a whole bunch of Clean History Zealots like myself, this is the rule most likely to be left out. But it's important!
Note that I'm **not** saying you shouldn't have some kind of code review process. Quite the opposite, you must! Even if you're working on a small project by yourself, you should have some kind of careful self-review step before you merge anything into master.
How to do code reviews like a human [Michael Lynch]
As part of that code review process, you're going to make edits to your commits: fixing typos, changing names, maybe some refactoring. Maybe in your first draft of the commit, you forgot to update some test cases. Your reviewer (or your CI) points this out, and you duly update those test cases. Everything's great, and now you're ready to merge your change! But: you don't want each of those drafts showing up as distinct entries in the final git log! It's still a single logical change, deserving of exactly one self-contained commit in the history â it just took you a couple of tries to get that commit just right.
Now, you *might* want a record of all of those drafts, and the back-and-forth comments between you and your reviewer, to be recorded *somewhere* for posterity. But the git log is not the right place for it! Instead, make sure that your code review tool is itself long-lived and searchable (archived GitHub pull requests, mailing list archives, etc).
Some of these rules might seem draconian, especially if you're used to a âjust press the green button alreadyâ workflow. If you're not worried about keeping a clean history, that's totally fine! But if you are, I think this is the minimum set of rules that you should strive to follow.
Since git was originally developed to track changes to the Linux kernel, and the kernel has a well-earned reputation for keeping a very clean history, it makes sense to look at the kernel's development workflow.
The biggest and weirdest commits in Linux kernel git history [Gary Bernhardt]
A guide to the kernel development process
Linux is a big enough project, with a large enough population of contributors, that there isn't really a single Grand Unified Process. That said, kernel patches are typically reviewed via regular emails sent to mailing lists. This, by the way, is the reason for the Generally Accepted Rules for commit messages â it's so the end result looks ârightâ when formatted as an email message! There are even several git subcommands (which you probably never use) specifically designed for this kind of workflow: âgit format-patchâ, âgit send-emailâ, âgit applyâ, âgit amâ.
How patches get into the kernel
How does it work in practice? Let's say you've developed an interesting new feature, and you've structured it using three commits that build on each other:
To get your new feature reviewed, you send each proposed commit as a single email. You construct the emails so that later commits in the series look like replies to earlier commits. That way, the entire series shows up as a single thread in your email client. Other developers review your code by replying to those same emails, using perfectly standard email clients.
At this point, the author will make changes to their code based on the comments from the reviewers. And here is where Rule 4 comes into play â the author _doesn't_ add new commits to their local branch titled âEdit typosâ or âAddress comments from Sueâ:
Instead, they'll rewrite the existing commits on their local branch (using âgit commit --amendâ, for instance, or the various flavors of âgit rebaseâ), producing a _completely new snapshot_ of commits:
They'll then email that new snapshot to the same mailing list, just like they did with the original one.
This process repeats until consensus is reached that the commits are ready to be merged. At that point, one of the subsystem maintainers responsible for that section of code will merge the code, using âgit amâ or âgit fetchâ + âgit mergeâ:
This process results in a clean history that satisfies all four of the rules listed above. The reviewers themselves ensure that Rules 1-3 hold, and the fact that reviewer feedback is incorporated by **locally rewriting history** ensures that Rule 4 is followed. The resulting git log does **not** contain any record of the back-and-forth that occurred during review; if you want to see that, you have to dig through the mailing list archives.
For better or worse, âmailing lists and the command lineâ aren't a good fit for most projects these days, and GitHub has won the resulting Great Code Hosting Wars. Is there an easy way that we can maintain a clean history, in the style of the Linux kernel, following the Four Rules, while using only the core tooling provided by GitHub? Now that GitHub supports squash merges, the answer is a resounding âmaybeâ.
Squash merge announcement [GitHub]
Before the advent of squash merges, the only option for incorporating the contents of a PR into master was a merge commit. This actually didn't seem so bad, since under the covers, it's just a âgit mergeâ of the PR's feature branch â exactly what the maintainer would do in the mailing list model!
About pull request merges [GitHub]
But! There's a disconnect, which is that the mailing list model also relies on locally rewriting history to address any changes suggested by your reviewers. You can do this with GitHub PRs â rewrite your commit history on your local copy of the branch, and then force-push to GitHub. For a good 10 years of my life, this is exactly what I would do!
However, GitHub strongly discourages you from force-pushing, even to a PR feature branch. Having worked recently with other code review tools (like Gerrit and Phabricator), I eventually saw the error of my ways, but only after yet another perfectly good review comment disappeared into the ether, because it was linked to a commit that had been rewritten.
The key insight is that you're using a GitHub pull request to track two distinct things:
In the mailing list model, these two things are tracked separately â one using git commits, and the other using emails and replies. In a GitHub PR, however, **both are tracked using git commits**. And that makes it very easy to conflate the two.
So mentally, you have to teach yourself to separate the two. You must create **exactly one PR for each single commit** you want to appear in the final commit history:
single PR for each final commit
And that PR's feature branch should track your interactions with your reviewers:
Even though as a fellow Clean History Zealot, it feels like fingernails on a chalkboard to create a commit labeled âFixed some typosâ, **that's what you have to do**, because that's the most accurate description of how the new PR snapshot differs from the previous one! And luckily, with a squash-commit, **none of those commits** will end up in the permanent history:
As I've described them so far, there's one remaining discrepancy between the email-based workflow and the GitHub squash-merge workflow: _series_ of commits. With the email workflow, it was possible to implement a larger feature as a series of smaller steps (individual emails for each small commit, threaded together using email replies), and importantly, **to have the entire series reviewed as a unit**. Reviewers would reply to the individual emails to make comments on the small commits, and the patch series as a whole would only be merged when **all of the commits in the series were approved**.
For a long time, I was confused about how best to accomplish this on GitHub, because of the same misunderstanding about what GitHub PRs are meant to represent. A PR can contain several commits, and so my natural inclination was to use a single PR to represent the entire patch series. You would see each of the individual commits in the PR, just like you would see in an email thread. Moreover, the force-pushes that I was using to update the contents of the PR lined up with the local rewrites that I would've performed in the email workflow.
But we've already seen the problems with force-pushing to a PR branch, and how we should use the commits on a PR to describe _code review_, not the _history of features_. So is there a way to author and review a patch series using the squash-merge workflow?
Unfortunately, the answer is ânoâ â at least, not without paying careful attention to what you're doing. You can use a GitHub issue to describe the patch series, and have separate PRs for each of the commits that you eventually want to appear in the history. The problem is that you can't (easily) have all of those PRs out for review at the same time. The problem is twofold:
You can use the base branch of the PR to take care of the first part, marking the âearlierâ PR as the base of the âlaterâ PR. If you do this, GitHub will correctly ignore any commit on the base branch when determining which commits belong to the later PR, and when rendering the diff view. But the base branch also determines which branch the PR would get merged into once it's approved, which isn't quite what we want. Grayson Koonce has described a way to make the merging work, though it's not *quite* the result that I'm looking for: you end up with a single commit in the git log, containing all of the changes from all of the commits in the series. I created the separate PRs in the first place because I wanted each one to show up separately in the final history.
Changing the branch range and destination repository [GitHub]
Stack pull requests [Grayson Koonce]
Since Grayson wrote his article, GitHub has added support for changing the base branch of a PR. This gets us _so close_: you'd mark the earlier PR as the base of the later PR. After everything is approved, you'd squash-merge the earlier PR into master. And at that point, you'd update the later PR so that it's base is now master. There are two problems: one is that because we squash-merge, the commit that got merged into master for the earlier PR isn't *quite* what's in the history of the later PR, and you'll have to perform a couple of merges (one of them using `--strategy ours`) to get everything to line up. And more importantly, you can't currently change the base _repo_ of the later PR, which means that your feature branches and the master branch that you want to merge into have to be in the same upstream repository. If you're following the usual strategy of editing feature branches in a personal fork, and creating pull requests in the âupstreamâ repository, branch changing won't work.
Changing the base branch of a pull request [GitHub]
So unfortunately, the current best practice is to not send out your later PRs for review until your earlier PRs have been approved and merged; or alternatively, for your reviewers to just deal with the fact that later PRs will include changes from earlier PRs in the series.
To solve this for real, GitHub should separate the current base branch concept into two pieces: The **base** would be the branch this PR should eventually be merged into (i.e, always âmainâ unless you're doing something weird). The **predecessor**, on the other hand, should be the âearlierâ PR that this one depends on. The âFiles changedâ tab would only show the diff between the predecessor PR's branch and the current PR's branch. GitHub wouldn't allow you to merge the PR until the predecessor was merged. But when everything is ready, it would merge the PR directly into the base branch.
Or, of course, we could just all go back to email patches around on mailing lists.
Thanks to JesĂșs GarcĂa Crespo for comments and corrections.
