GitHub Pull Request Formatting
Introduction
What makes a good Pull Request?
The answer will depend on your team, and to a certain extent your organisation.
Currently I work at BuzzFeed, and prior to that I worked at the BBC.
Both have a large number of engineers and teams, but due to different organisational structures they have differing opinions on what constitutes a good pull request format.
These opinions aren’t happening at the organisational level either, but are very much ‘team’ specific. Teams work differently and so have different needs.
Below I discuss some ideas around what I’ve used in the past and what I’m using today and I’ll leave it as an exercise for the reader to determine what parts they decide to takeaway with them.
Why
Probably the most important part of a pull request is understanding why the change is needed in the first place.
What exactly are you changing and is it even needed?
Are you solving a problem that promotes a real business requirement, or are you just adding a nice feature that doesn’t actually serve to improve the end user’s experience?
Maybe the feature you’re adding is an internal improvement (e.g. refactor, dev tooling etc). That’s fine and I guess it comes down to how your team prioritises its work.
But taking a moment to stop and think about a new code change before you start working on it, is an important step to take.
Size
Pull requests should be small.
A large pull request that touches many different files across a project and has many different side effects, outcomes and responsibilities is extremely difficult to reason about from the perspective of the person reviewing the code.
Small pull requests allow for quicker reviews and merging. It promotes an iterative approach to implementing new features. It also helps to avoid conflicts when merging or rebasing.
We’re also able to ‘fail fast’ if our work priorities change and we need to switch gears or change direction altogether. We’re not trying to solve every problem all at once.
Process
Pull requests should be opened almost immediately, to allow for team feedback and help in direction (depending on what you need).
For me, this typically means making a single, small change and opening a pull request around it.
Once the pull request is open I utilise ‘labels’ to signify the state of the pull request: wip (work in progress), rtr (ready to review), rtm (ready to merge), help (I actively want engineers to chip in early and help me flesh out the design if they see something wrong).
I ensure I have a good pull request description that indicates what the problem is that the pull request solves and how it solves that problem. In markdown that would look something like:
**Problem**: ...
**Solution**: ...
**Notes**:
...
**Todo**:
- [ ] ...
- [ ] ...
- [ ] ...
**Screen Shots**:
As you can see above, this is a generic template. I like to have the problem and solution lines to be very concise and not to take up multiple lines.
The ‘notes’ section can be multiple lines so I keep it as an isolated section. This section can also include things like automated code linting or test suite coverage results.
The ‘todo’ section helps me to keep track of what tasks I have in order to complete this pull request, but it also helps other engineers to understand my thought process and see where I might be going before I even get there (allowing the team to ask questions or make suggestions as per the help label).
Finally, the ‘screen shots’ section is useful for those unfamiliar with the side effects of the change to be able to visually identify where the change appears or what it looks like. This isn’t always necessary, depending on the code change being made, but is useful for UI changes.
Communication
It’s important to notify fellow team members that your pull request exists, in order for them to provide appropriate feedback.
You can manually @<username> people within a comment in the pull request, or you could create a team in GitHub and then @<team-name> instead (helps especially for teams who like to rotate members across other teams within an organisation).
Merge Strategies
I’ve written in the past about different ‘git merge strategies’. My preferred way is to git merge --squash, and luckily GitHub’s UI provides a ‘one click’ way to squash merge a pull request.
That’s what I suggest using for merging pull requests into master, but you may have different requirements in your team/organisation, so pick whatever works best.
Conclusion
So this has been my preferred approach to creating a good pull request.
To recap, here is a top level look at the structure and concepts I suggest:
- Small pull request
- Utilise labels to indicate status
- Consistent formatting:
- Gif (optional)
- Problem/Solution
- Notes (optional)
- Todos (optional)
- Screen shot images (optional)
- Agree a merge strategy for team consistency