Dave Sníd

Unwritten social rules around GitHub PRs

September 21, 2023

Over the last 15 years I’ve written and reviewed over a thousand GitHub PRs in open and closed repositories. As a designer who codes I often deal with substantial project changes that not everyone agrees with. I’ve come at a PR from every angle over the years and have learned to vary my strategy based on the sensitivity of the project and / or the person. Here are some tips learned:

For authors

  • If you don’t want feedback yet, make it a draft.
  • Title it for history and be clear. Previous PRs are mostly only seen when something breaks. Simple sentence starts like “Adds”, “Changes” and “Fixes” help.
  • If the PR changes UI include before and after screenshots. For complex interactions, embed an MP4.
  • A PR that adds documentation has a higher likelihood to be received favorably. Likely when writing docs, you’ll change your code and make it easier to understand. PR descriptions fade into obscurity, but docs help people forever.
  • Put a to-do in your description if there are items left to get done. If you decide not to get to them in this PR, cross it out (don’t delete it) and give a reason. This helps prevent “did you think about…” commentary.
  • Use “feature branches” to break complex projects into smaller reviews. Each smaller PR can merge into the top-level one. Not many people have the stamina to review past 1000 lines of code.
  • Ask for help. Use code comments before review to mark pieces of code you’re not happy about. This gives reviewers a place to focus.
  • Tone is hard to pick up in writing. If you assume a negative tone from the reviewer, ask to have a conversation instead of fighting back over GitHub. If a thread goes beyond three comments, it’s time to stop writing.

For reviewers

  • For fuck’s sake, if they made it a draft, don’t comment unless its about being excited for what’s coming.
  • Phrase your language as a suggestion rather than a demand. Create a choice for the author.
  • If you are personally requested for review (rather than through a group) it is in your best interest to review it before doing your own work. Doing so will earn you a work-buddy.
  • Follow up with a quick approval if they take your suggestions. The speed at which you look at a PR the second time is typically more important than the speed you look at something the first time. They did what you asked, don’t make them wait.
  • Small changes or “nits” are worthless unless your org has a published style guide. Provide a code suggestion (using the tooling) or make a PR against the PR “Here’s some small stuff I noticed, take it or leave it”. This lets you come in friendly, rather than accusatory. Make it easy for them.
  • Always ask to commit to the PR before doing so. Making a separate PR that targets their PR is the easiest option. This loosens after a relationship builds.
  • Watch the repo (RSS or email), and read changes as they happen. It’s a lot of work, but you will have more context (how long, what changed, the author’s style) before you get to the final PR.
  • If you have product or architectural concerns about the PR, immediately stop using GitHub and start a face-to-face conversation. Depending upon how that goes you can always add “Discussed the following over zoom” after an agreement has been made.
  • We are sensitive humans. Use rejection for hard “do not merge in its current form” and use comments for “suggestions to improve” reviews, trusting to the social culture of your group.
↩ More posts