When you first started learning to code, your projects tended towards solo, short-term efforts. You didn't have any need for documentation. No one was ever going to maintain your code. No one was even going to look at it unless to assign a grade.
Projects in industry come with much larger teams and much larger timescales. Your teammates will need to understand, extend, and maintain your code long after you've moved on. The simplest and most common way to supply the necessary documentation is in your pull request (PR).
So how do you write a good one?
Not a good one
It starts with code
The ideal PR represents a small, conceptually independent, code change (the diff). It might be a bug fix, a new feature (or a part of one), or some focused refactoring. Reviewers should be able to understand what the code does without repeating the work that you've already done. Large or mixed diffs make this much more difficult.
It can be tempting to batch several changes together for "efficiency". If you put five bug fixes together in one PR, that appears to save you time. It can also appear easier to create a single PR containing an entire feature instead of several PRs along the way. Only one PR to write. Only one code review approval to chase.
But these are false efficiencies and can hurt you in many ways:
Delayed approval. Reviewers tend to avoid difficult PRs.
More bugs for later. Reviewers are prone to miss issues in complex PRs.
Increased chance of rewrites. Reviewers will find structural issues at feature complete instead of checkpoint #1.
Need to split up the PR anyway. Reviewers may insist on replacing your complex PR with small, conceptually independent PRs.
Inability to revert independently. If you must later revert one of your five bug fixes, you may be unable to do so without reverting the others.
So keep it short and as simple as you can.
Documentation
Now that we've scoped our diff well, we can consider the documentation around it.
Abstract away the tools and you have two pieces of documentation to consider:
The description of a desired result. Usually a bug to fix or a feature to implement. This description includes any back-and-forth discussion before writing the code.
The description of the attempt to produce the desired result. What you considered, the approach you chose, and how you know it produces the correct result. This description includes any back-and-forth discussion of modifications to the diff as a result of code review.
In the short term, this documentation supports your ability to ship the desired result.
Without a good description of the desired result, you can't be expected to accurately build it.
Without a good description of the attempt, the team can't know if you achieved the desired result without duplicating your efforts.
In the long term, this documentation supports your team's ability to investigate, debug, and make further changes.
Without a good description of the desired result, maintainers may misunderstand what they need to preserve and break it.
Without a good description of the attempt, maintainers may waste their effort with approaches you discarded or investigate the wrong parts of the code.
Now that we understand the "why" and the "what", we can add the tools (the "how") back in. The pre-change documentation lives as a ticket in your ticket tracking system (Linear, JIRA, GitHub Issues, etc). Writing a good ticket is a topic for another day. The post-change documentation is your PR.
So when writing the PR, we should remember what kind of documentation we need:
You already have a ticket explaining the desired result and the PR should not duplicate this except to include a high-level summary.
You already have the diff and the PR should not duplicate this except to provide a high-level summary.
The PR should provide an explanation of your approach, alternatives you considered, why you made the choices you did, and how you know it works.
If the goal is to provide this context, what information should we include and how should we organize it?
PR Templates
If you are lucky, your repository includes a PR template. This is a file with some standard sections that are automatically copied into all new PRs.
A standard format decreases the chance you will omit important information and makes life easier for reviewers. Sometimes the template only has simple sections (eg. description, testing). Sometimes the template has checklists for the developer (eg. added unit test, added feature flag). Sometimes the template has checklists for reviewers to understand scope (eg. adds new API endpoint, adds new database stored procedure).
Just as important, having a template indicates that the team has discussed what they feel is most valuable in a PR and come to an agreement. Much like coding style discussions, individuals can have strong opinions about what to include. Finding the right balance between necessary documentation and unnecessary red tape requires effort.
If your repository doesn't have a template, consider suggesting that you create one for your team. It’s easy to set up, and nothing says "growing towards mid-career" quite like improving the development process for the whole team. The hardest part is getting the team to agree on what goes in the template.
But we didn't answer our last question. What information should we include?
Good PR Sections
Although the names can differ, information in a PR often falls into six categories:
Title
Quick Reference
Motivation
Changes
Validation
Checklists
Remember that we want a balance of the right amount of documentation. A one-line visual change will call for a much simpler PR description than major feature work.
(If you need a refresher on the mechanics of creating a PR in GitHub, see Creating a pull request)
Title
A good title will include the ticket tracking number and a succinct description of the change. It should be easy for a reviewer to find the ticket from the PR. It also should be easy for a future investigator to skim through a list of PRs. This can be surprisingly common when narrowing down a regression in a release or walking through the history of an obscure section of code.
Poor: Add new setting
Good: PROJ-123 Add selector for new Foobar setting to Settings modal
Quick Reference
At the very top of the PR body, list anything that might be of value to an on-call engineer who thinks your PR caused a site outage.
This may include:
A link to the ticket.
A list of any feature flags that control your change, with a one-line description of what they do and when to use them.
A list of any telemetry markers in your monitoring systems, with direct links if your system supports it.
Whether it is an outage or a reviewer wanting to run your changes locally, having these values up front can save a lot of time searching your diff. And maybe keep them from calling you at 3 am to explain how to disable your feature.
Motivation
This section answers the questions "Why do we need this change?" and "Why did we do it this way?"
Start with a high-level summary of the ticket (aka the desired result). Then describe what alternatives you considered and any important context in how you decided. Remember that you've been thinking about this problem for a while and your readers have not.
If this is a UX change, before/after screenshots can go a long way toward explaining and can save reviewers from needing to pull down and run your changes.
If you ran into issues or there are parts of the change you are unsure about, prominently call it out. This will guide your reviewers to pay close attention to those areas. Sometimes devs are concerned that this will reflect poorly on them, but it is an important part of the learning process and no decent reviewer will hold it against you.
Example:
The new Foobar setting should be editable by the user in the Settings modal.
Unlike the other settings, this one has a dedicated API call. This complicates saving multiple changes. We chose to introduce extra complexity in SettingsModal.tsx to keep the UX consistent for the user.
Changes
This section answers the question "What did we change?"
Start with a high-level summary of the contents of the diff. You don't need to go line-by-line or file-by-file, but a bulleted list of themes can be helpful.
Example:
Add selector in Settings modal to edit Foobar value
Update loader and error handling logic for concurrent API calls
Add state, reducers, and thunks for new Foobar API call
Validation
This section answers the question "How do we know this change works?"
Describe how you tested the changes. This might include unit tests, integration tests, or manual testing. In a complex system, features can interact in unexpected ways. Knowing your testing approach allows a reviewer to ask questions about other areas that need to be checked.
Example:
Added unit tests for new reducers
Saved each Foobar option
Saved Foobar with and without other setting changes
Forced an error from the Foobar API and verified the error in UI
Checklists
When filling out a PR, you only need to check the relevant boxes.
When creating a PR template, consider the following examples of important callouts. Include any that are valuable for your project and team.
Type of change (bug, feature, refactor, etc)
Downstream changes needed (client changes, public documentation updates, etc)
Testing (all tests pass, added new tests, etc)
Standard additions (includes feature flag, telemetry, etc)
Pre-reviews (design approval from security, database administrators, etc)
Just Getting Started
Hopefully that helps you improve your PR. As with any kind of writing or documentation, it takes time to find the right balance between too much and not enough.
The future will thank you for taking the time to do it well.
Speaking of the which:
Did I omit anything that you like to see when reviewing PRs?
