My Ideal Git Workflow

I went to go look at another team’s code repository recently to try to find something and I couldn’t help but notice that their git history was a mess of merge commits and confusing commit messages.

Now, there aren’t hard and fast rules for how to git (no, really.), but I am of the opinion it is generally helpful to keep a clean history to tie changes in individual files to tickets. If you can do this with a single merge bubble per feature or something else, more power to you.

What follows is how my team and I prefer to maintain our git history.

Avoid Merge Bubbles with Rebase

Instead of just merging your branch into master after a Pull Request closes (never trust Github’s automatic merge!), try this instead:

# make sure your local copy of master is up to date
git fetch
git checkout master
git merge --ff-only origin/master

# rebase your feature branch against master
git checkout my-feature-branch
git rebase origin/master

# this rebase may have caused conflicts
# just resolve them like you would a merge, then...
git add .
git rebase --continue

# update your branch after rebasing, for posterity. 
# you need to force the push, because of the rebase
git push -f origin my-feature-branch

# finally merge the branch cleanly
git checkout master
git merge --ff-only my-feature-branch
git push origin master
This process completely avoids merge commits and the associated bubbles. Your git log will always show a single unbroken thread for whatever branch you are on.

Advanced Tip – The Release Branch

If you use a secondary branch like “release” or “develop” for building to a Test environment (vs master branch for QA/Prod), the easiest thing is to just merge the branches twice. Once on your test build branch (without rebasing). Then after the ticket is approved to go up the stack, rebase against master and merge, as shown here. Then rebase your test build branch on master (like it was a feature branch), which will remove any merge commits you have added.

Commits and Commit Messages

There are many ideas of what makes a good commit message, but focusing just on the initial subject line, I recommend:

[404] Create default error page

or, if you are on JIRA instead of Github for issue/ticket numbers:

[PROJECT-404] Create default error page

The message should be present tense, clear, and short. The textual tag of the issue/ticket number is optional, but very helpful when scanning a git log.

As far as commits themselves, I would recommend committing as often as possible during development, and don’t worry over much about messages at this point (especially longer descriptions). At least commit once per day, if only to be able to push your changes up.

When you are ready to do a Pull Request, use git rebase -i HEAD~10 (or how ever many commits you have) to merge several of them together into logical pieces. Feel free to rearrange or edit commits at this point, reducing the number to minimum logical groupings. For smaller features/bug fixes, a single commit at the Pull Request stage is recommended.

After the Pull Request, squash all of your commits together, unless your Pull Request comprises multiple unrelated shippable features, then squash the commits down to one per shippable feature.

On Code Reviews

Or, How I Learned to Stop Worrying and Love the Squirrel


In a couple weeks I will celebrate 7 years of professional development. In those years, I have committed code to production in a variety of ways. I have SFTP’d files up to a live server. I have merged feature branches to a master branch. I have committed directly to master in a CI/CD workflow. I have participated in several iterations on git flow style branching. I have rebased live history, sometimes to great effect (removing an excessively large data file from our code’s history), and sometimes to great pain (having to manually rebase a dozen different branches to remove a couple commits that weren’t ready for prime time).

For the first several years of development, these code commits were largely autonomous and lacking any significant review. If they were reviewed, it was only to make sure the new code worked as expected and didn’t have any hidden problems, because we weren’t doing automated testing either. I was growing in skill by sheer attrition, sometimes debugging issues in production, sometimes spending days trying to reproduce issues locally, but only getting fundament development guidance from the occasional blog post or podcast.

In the past couple years, it has been a different story. My previous job was at ePublishing, an all-remote team with excellent developers and well defined standards and practices. It was here I was first shown what a pull request could be: with useful feedback on best practices and standards and multiple people participating and eventually signing off on each review. One rule was made very clear: on any non-trivial change, two :shipit: 🐿s (or :+1: 👍s) were required. This made the call on what was approved very easy to tell, though it sometimes left the PR discussion a little lacking.

A year ago today (8/3) I started working for Ramsey Solutions, a company that does “work that matters” in areas like personal finance and business leadership. This is a great company with great team members, but when I started I noticed some gaps in the code review strategy (or lack thereof). I was back to the wild west of optional reviews, and fast/shallow analysis of the code when it did get reviewed. So I resolved that (at least on my team) things were going to be better. With the help of my leader our team started using the two squirrels strategy I was used to on all our PRs going forward. This helped quite a bit on it’s own, because it got more people reading the reviews, which in turn gave more chances for discussion. But as the team grew, we didn’t have any consistency with the amount or severity of feedback we were giving besides the final :shipit:. In addition, we sometimes were giving “half-shipits” via the :+1:, which led some to wonder whether the code was good to go or not. Several of us on the team also started inserting things like “⛵️-it” or “🐏-it”, which, while amusing to punny people like me, really confused our newer team members.

A few months into this practice, at a quarterly team planning session, we discussed “the PR problem” and eventually codified some new rules and concepts.

Firstly, we decided that you could no longer leave :+1: (or anything else, no matter how punny) as a final meaningful response on a PR. If you are going to participate in the code review at all, you must drive the PR forward until you feel comfortable with a :shipit:. To this end, you might need to add tickets to the backlog to fix things that could not be addressed in this PR. These tickets are new technical debt for the project and should be remedied as quickly as possible so that paying down the the interest doesn’t overwhelm new work.

Secondly, we decided to call out (literally, label) four severity levels of comments you can leave, and we clearly defined each level:

  • The :shipit: is the first and most obvious severity level and means you are 100% OK with this code being in production as-is.
  • The next severity is blocking which, as its name implies, points out something that prevents the author from giving a :shipit: until resolved (either in this PR or as a new ticket). These can range in cause anywhere from critical logic issues to coding/design standards violations. As long as you won’t give approval until it is resolved, it is blocking.
  • The middle severity is rocking. These are the “nice to haves”, things like refactoring messy code or following a standard that is not officially codified, or less important.
  • Note: If you only give rocking feedback or below, you should also give a :shipit:, because you didn’t find any blocking problems. The committer should read all these comments and at least consider whether now is a good time to make the change, or even if they agree with it (discussion is encouraged!). These comments can also be sources of tech-debt tickets if the committer agrees the change should be made, just not right now.
  • The lowest severity comment is talking. These are questions and general comments on the code, including :+1: as praise for a particularly well written line of code (or refactor/deletion of a poorly written one!). These often require no action unless there is a question attached, but should still be reviewed before merging.

Third and finally, we started leaving comments on specific lines of code wherever possible so that when the comment was addressed the discussion disappeared, hopefully leaving a fairly clean diff at the end (:shipit:s being the one kind of comment that is encouraged only at the top level).

After this first draft of rules was decided at our quarterly planning session, the quality of our code reviews went up drastically. Everyone committing code knew exactly what to expect when they submitted their PR, and conversations were much more focused because the original commenter gave their feedback a reasonably severity level. A lot of the discussion around rocking and talking comments has helped us to codify new standards/guidelines for our code base, where as the non-committal :+1: which might have been given before would often fail to generate a conversation.

After we had been using these conventions internally for a while (and as our team grew) we started using them when giving feedback on code reviews outside our immediate team. This caused some confusion initially, so one of our team’s developers gave a short talk about the convention at the next technology team meeting (our team is about 8 people, but there are over 80 people in the technology space here). Other teams then started adopted the standard, some of them replacing the English terms of blocking, rocking, and talking with emoji for brevity: 🚫, 🎸 (or 🤘), and 💬. Our team had actually discussed this but decided against it at the quarterly meeting because it seemed too finicky, but more power to the teams that decided to use them as long as they are consistent and clear.

I have really appreciated the level of feedback we have on code reviews now, and of course we are always looking for more methods to improve the experience even more. But discussing processes like this one with your team, even if you think you are doing everything all right, can be very instructive because the discussion itself can help further define things even if no actual changes are needed.

So go out (or possibly stay in) and improve your workplace environment today!


PS: After several months of using the new conventions, our team had a chance to work on a large project under a tight deadline. Since we were spread so thin, our fearless leader (er, our Director of Technology) decided to take a hand in approving PRs. He declared by fiat that his :shipit: squirrels were worth two of ours, so we lovingly created a special :leadershipit: emoji on Slack just for him.