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.
Clearly documented PRs are an important part of our engineering culture
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.
We want to help reviewers engage with the changes
When we put a PR out for review, we’d like the reviewer to:
Get started reviewing! We're heavy Slack users and a post in a team's #foo-pr channel should make them feel confident that it’s a well-thought out PR that won’t take too long to review.
Understand the context and thought process behind the change.
Feel confident that the PR doesn’t introduce bugs.
If we nail all of this, the PR will sail through the review process quickly.
We want to help our future colleagues and ourselves
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.
Following an information hierarchy
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.
Level 1: The PR title
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.
Level 2: The PR description
The description makes up the bulk of the PR documentation and we use it to share:
The full background on the problem area
The approach we've taken to address that problem
Any manual testing steps (to complement the automated tests in the PR)
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!
Level 3: Comments
There are two kinds of comment and we use them in slightly different ways:
// code commentsto explain to both the PR reviewer and future readers of the code why the code behaves a certain way, or to give clues about non-obvious behaviour
Inline GitHub comments can give PR reviewers pointers on things like areas of the PR to focus on, things the author felt unsure about, or why we've made specific changes
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.
Level 4: Links to other relevant resources
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'.
This hierarchy empowers people to only go as deep into the PR as they need to
Depending on their needs, readers will delve into different levels of a PRs information hierarchy:
If they just want to understand if a PR in #foo-pr is relevant to their work, or whether they feel well-placed to provide a review, they may just look at the PR title (Level 1)
If they want a broader overview but aren’t interested in the specifics of the implementation, they’ll look at the title and description (Level 1-2)
If they’re reviewing the code, they’ll most likely take in the title and description, then read the code, together with the GitHub and code comments (Level 1-3)
If that’s still not enough they can look at the further reading resources (Level 1-4)
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.
Information hierarchies aren't just for PRs
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).
We apply our 'tone of voice' when documenting PRs
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:
inclusiveness (our work needs to be accessible to a broad group of diverse engineers across the company)
using short sentences
preferring conversational language
We apply these principles when writing PRs too.
How do you document PRs?
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.