Documenting pull requests is as important as writing good code

All code at Monzo is peer-reviewed. We manage the review process through pull requests (commonly known as "PRs"), a widely-used GitHub feature where you bundle up small incremental changes. Another engineer can then go through the changes and do their best to catch bugs, question the rationale, propose structural changes, and ask for things like more test coverage. There are many things to consider when writing a great PR to make it easy to review (and to be confident that we won't break something when it gets deployed!). In this blog post, we're going to focus on how we structure PR documentation.

We have a philosophy that the reviewer of a change has just as much responsibility for it as the author, so making it easier to understand and interpret the work is crucial. All new engineers at Monzo are encouraged to document their PRs well from their very first change. We have several motivations for this.

When we put a PR out for review, we’d like the reviewer to:

If we nail all of this, the PR will sail through the review process quickly.

Reading a well-documented PR can help someone in the future understand how something has evolved, even if the specific change itself happened a long time ago. In my three years at Monzo I've often needed to understand not only what the code does right now, but why it behaves that way. Otherwise I might make some naive changes that break invariants I'm not aware of.

To achieve both of these goals, we want to make it as easy as possible for someone to review a PR.

Not every engineer needs the same amount of detail about a change so we organise our PR documentation into a series of levels. Each level builds on the last to help the reviewer build more a nuanced understanding of the change.

The title is the highest-level statement of what's changing, where we try to capture the essence of what we're doing and, ideally, why it’s important, all in one concise sentence!

Here's an example:

A title focused on what the change is would usually say something like "Display ProviderLabel instead of Provider". We avoid titles that state the name of a field, function or file, without summarising what the change actually achieves.

The description makes up the bulk of the PR documentation and we use it to share:

Here's an example from one of our open sourced libraries:

And here's another example:

We don't need to always write a lot of text in these descriptions. Sometimes a diagram or a screenshot and a link to related documentation is enough context:

If it is worth writing a longer description, we sometimes use subheadings to make it easier to read. This introduces a mini information hierarchy inside the PR description!

There are two kinds of comment and we use them in slightly different ways:

We tend to ask ourselves, 'Once my code is merged, will this comment still be helpful?' We find commentary on how the code used to behave can be really insightful and helpful during PR review, but isn't always as helpful after we've merged the changes.

We often make lots of little changes so it can be helpful to provide much more context to help the reviewer understand how this change came about. For example, describing the change in relation to a wider project or pointing to a team discussion.

We usually embed links to Slack threads, technical proposals, or other documentation in the body of the description. If there's no natural home for these links in the body of a description, we list them at the end as a sort of 'appendix'.

Depending on their needs, readers will delve into different levels of a PRs information hierarchy:

Because of this, it’s important to think about what information would best help the reader. We tend to ask ourselves: "Is this best captured in the PR description, in a GitHub comment, or a code comment?”

In all the levels, why is more important than what. For instance, "rename assets from HTML to GOHTML" is a what description – we’re renaming some files, but it doesn’t tell us why it’s desirable to do so. Changing this to "change extension of Go HTML templates from .html to .gohtml, for better syntax highlighting" turns it into a why description and helps others understand the motivation for the change.

I've just discussed PRs here, but we apply this idea of layering information for everything we write. For example when we send important posts in Slack we might start with a headline in bold (Level 1), followed by a paragraph that summarises the ideas (Level 2). If we've got intricate details to share, we might put them in a thread (Level 3), and provide links to other resources (Level 4).

Monzo has some 'tone of voice' guidelines that we use to structure all writing at Monzo. This includes the writing found in screens in the Monzo app, conversations with customers, or in our internal discussions in Slack. Our tone of voice covers things like:

We apply these principles when writing PRs too.

Every engineering organisation approaches PRs in a different way so we'd love to hear what you like to see in a PR as a reviewer, or what you include as an author.

If you like the sound of how we work, check out our current engineering vacancies on our careers page.