Committing code at The Gnar Company

by Mike Stone

TL;DR

There are a few categories of code commits, each with their own goals.

• Development commits - useful to the commit author; get squashed/re-worded before code review.
• Code review commits - useful to the PR reviewer. They provide helpful context on the problem and solution, or a description of the feature to aid in review. After receiving PR review, the commits to address specific comments may not take the same form as the initial code review commits but instead include a short message, similar to a development commit, that is intended to be squashed into a production commit.
• Production commits - useful for future-selves. Similar to (and often exactly the same as) code review commits, they detail the reason for the code change to provide an understanding of historical decisions made when the code needs to change in the future.

Short-enough; Gonna-read

Version control enables us to track changes to the codebase. It provides a number of other benefits, such as:

• helping engineers to save their progress locally as they work towards a solution.
• aiding in the debugging process by providing context for development decisions when inspecting historical changes.

This article will focus on three categories of commits: development, code review, and production commits.

Categories of commits

Development commits

Development commits are the local commits that an engineer makes while working on a feature, bug, or chore. They are made to serve the engineer's code contribution process.

The goal of development commits is to aid in the development of features. Given this goal and differences in personal work styles, the requirements of development commits are fairly loose. Commit messages are meant for the author’s eyes only and may not be helpful to other engineers. Examples include “WIP”, “Fix test”, “Make it work”, etc. These commits should be atomic to save valuable time in the development process when dropping/cherry-picking commits. They can be particularly helpful when spiking work so that the author is able to breadcrumb back if the direction changes.

Code review commits

Before submitting a branch for code review as a pull request, it is helpful for an engineer to group their commits into logical, atomic commits to help others who donate their time to review the work. Each commit should have a strong commit message and description to describe the work of that commit. This description should not simply reiterate the code changes that will be obvious to reviewers but should instead describe the problem that the code fixes.

To illustrate this point, let’s say we are building a product that has a feature of sending reminder emails to follow up on tasks. We discover a bug when inspecting our error logs that the reminder email service is occasionally failing. We are assigned the ticket and upon further investigation, we notice that the email is failing because it is trying to send itself to users that no longer exist in the database.

Our fix is a small code change, something along the lines of:

def send_email(user)
  return nil unless user.present?

  # ...existing code to send the email
end

We can commit this code however we want when working locally, but before asking teammates to review this code we need to provide more details. A commit message similar to “return nil if there is no user” doesn’t help because it’s obvious from the code diff that this is the change. Instead, we use the commit message and description to provide more information about the problem and solution. For example:

The goal of code review commits is to make it easy for reviewers to understand why the code change was made. They accomplish this goal by explaining said change next to the change itself, which provides useful context for reviewers.

Do not attempt to send reminder email if no user

This commit:

* Ensures we have a user before sending a reminder email

Why?

This change is intended to fix a bug in which we attempt to send a reminder email
that does not exist. This bug is due to the user having deleted their account before
the task reminder email is sent.

Moving from development commits to code review commits often requires rebasing, squashing, and renaming commits to prepare the code for review. The message of a code review commit is also likely to be different from a development commit. The message is a crucial part of providing the required context to the reviewer. Along with a message to describe the general solution, add a description that includes the sections “This commit”, which details the changes made to solve the problem, and “Why?”, which provides an explanation for why the change to the application is needed. Once the author of the pull request has received feedback from teammates, the PR comments can be addressed in commits similar to development commits (for example: "Address PR feedback") so that reviewers can see the specific changes that were made related to their comments.

“But", you say, "I link from my pull request to the ticket in the PM software and that has all the information anyone needs”. This may be true, but remember we’re making it as easy as possible for the generous reviewers of our code to make effective use of their time. Asking them to bounce back and forth between PM software and version control software isn’t ideal, and there may be more technical details in the code commit than are necessary to detail in the ticket in the PM software. Also, these commits often make it into the production branch of the repository and provide useful historical reasons for code contributions. In addition, PM software serves a different audience and there's no guarantee that your current PM software will continue to be used in the future.

Production commits

These are the commits that are merged into the production branch of the application and that contain the production code. The commit messages/descriptions of a production commit take the same format as code review commits, and the commits themselves may be exactly the same as the commits during code review. The goal of production commits is to help future-selves understand the rationale for past code decisions and contributions. Typically these commits are either the same as the code review commits, or are created from a combination of code review commits. They should encapsulate the code for the solution to a problem or a new feature along with a message that clearly describes the changes and the reason(s) for the changes. In the future, when there is confusion over a line of code in production, understanding why it’s there and if it really needs to be there becomes an easier task by looking at historical commits and their messages.

Other styles/flavors

PR review commits

Some companies prefer to add commits for work that addresses comments on pull requests. This style of commit message is a hybrid between development commit and code review commit in that we want to convey the reason for the change in the message but do not need to provide the same level of context in the description. If using this commit style, squash your commits appropriately before merging to production so you can still benefit from production commits.

Merge commits

This is a hotly debated topic and deserves its own dedicated article. I don’t want to diverge from the focus of the commit topic so I’ll save this for another day (yes, I’m copping out :smile:).

Conclusion

Be mindful of your future-self and teammates when committing code. Empathize with those you ask to review your code and try to make the time they donate to your work as efficient as possible. Taking the time to commit thoughtfully now will save you time and headaches in the future as complexity builds and the context for historical changes is lost.