3 min read

🏋️‍♀️ Pull Requests at Pactio

[Author: Will]

An important moment in the day to day workflow of any engineer is raising a pull request. This can be particularly intimidating when it's your first (or second, or third) at a new company. It's also hard to understand the kind of feedback people are used to, expect or will provide. As any knowledge is tribal unless written, we have decided to write down guidelines (as usual, not rules). This has been super helpful to onboard new joiners and get them used to work with us. Any new joiner will bring their particular experience and flavour to our team culture, so we make sure to revise these guidelines often. It's important everyone is onboard with them, otherwise they become dead documentation.

As usual, we have decided to share our current guidelines to help potential new joiners better understand what is it like to work at Pactio.

Requesting feedback

  • Write a short and concise description of the changes your PR makes, providing enough context for people to be able to give you an effective review. Here are some tips in case you were lacking inspiration:
    • Describe why are we doing this
    • Describe the approach we are taking and if we evaluated any alternatives
    • Beware that this description is mostly for posterity
    • Don't assume familiarity with the issue at hand
    • Provide links with good context (e.g. Slack conversations, any new issue created, Stackoverflow answers, etc)
    • You can find a great articles for inspiration at the following:
  • Favour small PRs
  • Be explicit about which areas you really want feedback on
  • Be explicit about when you want feedback, if the Pull Request is work in progress, leave it in draft or explicitly say so.

Offering feedback

  • Familiarize yourself with the context of the issue, and reasons why this PR exists
  • If you struggled to provide useful feedback because you didn't understand the context, be explicit about it
  • Give it your 110%. You are safeguarding your peers from making mistakes that would lead to incidents, they are trusting you to read their code carefully
  • Summarise your feedback and its tone in the review title. Receiving 20 comments with no context it's daunting
  • Find something nice to say. We always focus on the one line that we don't like and never appreciate the 300 lines that are great
  • If you ask for changes, be explicit about what needs to be explicitly addressed
  • Beware of abusing the Request Changes button. The intent you communicate is that you want to be the gatekeeper on the PR. This indicates:
    • You will promptly re-review the PR when changes are made
    • You want to see the changes through
    • If your feedback is more neutral, you can simply comment the PR.
  • If you wrote comments and Approved, make sure to summarise the intent of your comments and highlight their presence so your peers don’t miss them. Also worth checking if the PR has auto-merge enabled.
  • Explain your reasons why code should be changed. If it's a personal preference or nitpicking, say so. Prefixing the comment with nit. is an helpful way of indicating that
  • Ask, don’t tell (“What do you think about trying…?” rather than “Don’t do…”)
  • Favour code suggestions over describing code with words
  • If you are about to write a comment you wouldn't like to receive, consider giving it a few minutes before responding
  • Avoid using derogatory terms when referring to the work someone has produced.
  • Be humble (“I’m not sure, let’s try…”)
  • Avoid hyperbole (“NEVER do…”)
  • Be aware of negative bias with online communication. If your tone is neutral, it will be perceived as negative. Can you use positive language as opposed to neutral?
  • Use emoji or adverbs to clarify your tone.

Responding to feedback

  • Consider leading with an expression of appreciation, especially when feedback has been mixed. No one likes to displease a peer, and giving you that feedback must have been hard
  • Ask for clarification (“I don’t understand, can you clarify?”)
  • Offer clarification, explain the decisions you made to reach a solution in question
  • Try to respond or at least react to every comment
  • Link to any follow-up tickets ("Good call, would you block on this? We could otherwise address this in [issue number]")
  • Be explicit about a PR being ready for a re-review
  • Don't rebase between reviews. It breaks Github's changes since your last review, increasing the cognitive burden on reviewers