To kick things off, here are some thoughts on writing outstanding Pull Requests that I have previously shared with colleagues. The longer I’m a a developer, the more I see how much clear and informative PRs help the entire team.
We should strive to put up PRs that are easy to review and easy to test. The code diff and PR description on GitHub are just the start of a teammate-friendly PR.
Investing a bit of time upfront crafting a well-written, clear, comprehensive PR will save everyone time in the long run and will make it easier for the reviewer to review thoroughly and the tester to test thoroughly.
Precondition: A card with clear descriptions and up-to-date, comprehensive scenarios.
A “complete” PR has all of the following:
- A thoughtful PR description (more below).
- Code diff (automatically generated).
- Comments on specific lines of code to share helpful context with the reviewer, if appropriate (e.g., this line of code addresses this particular issue, this new method is needed for this reason). These comments act as guideposts that lead the reviewer through the work.
When writing your PR description, it can be useful to adopt the mindset of someone unfamiliar with the feature or task. In general, I recommend that the description contains:
- A link to the card.
- A statement describing the goal of the PR.
- Testing notes not covered in the card (e.g., relevant environment, specific state requirements such as particular accounts or stores, failure scenarios that may not appear in the card).
- Screenshots and/or gifs, ideally arranged in tables (e.g., before/after, different versions of flows).
- Additional helpful context for the reviewer (e.g., things you tried that didn’t work, issues that you observed that will be addressed in a later card). Think about what would make your life easier if you were the reviewer. You can also add this information as individual comments on the PR.
- Additional helpful context for the quality engineer (e.g., screens that are impacted by the changes, screens that should not be visibly impacted by the changes but underlying code has changed). Think about what information would help make testing efficient and thorough if you were the QE. You can also add this information as individual comments on the PR.
You shouldn’t need to explain your thought process across all of your changes. Focus on what’s unexpected, unusual, or complicated. Take a few minutes to examine the card. Has anything changed since it was written? Are any scenarios missing? Any differences across platforms that should be noted? Make any updates necessary to keep the card and the PR in sync.
If fixing a bug, what caused it? Explain the cause of the bug and how you went about resolving it.
Did you add functionality or fix a bug that isn’t captured in either the card or the PR summary? Make a note of that certainly in the PR summary and possibly in the card to maintain consistency.
Run through the relevant scenarios on your device or in an emulator. Do your layouts match the comps? Is there a necessary discrepancy? If so, make a note of that in the PR, and even better comment that you’ve already run the discrepancy past the designer. Is all behavior correct to the best of your knowledge?
What neat thing did you learn that would be good to draw attention to? What could other teammates learn from this PR?
Ok, now take a deep breath and submit your PR.