Back to the front page

How We do Pull Requests

AlphaSights Pull Requests

At AlphaSights we have a very specific way we like to do PRs and try to follow a set of guidelines to standardize the process. We aren’t dogmatic about it or anything, so if one of our “rules” doesn’t make sense we leave it out. However, for the most part we follow them, resulting in reviewers having enough information to give a solid review and reduce the likelihood of introducing bugs and/or technical debt. It also makes it easy for someone to revisit the PR months later and easily understand what was going on.

What We Include

  • Whatsup - explain the problem the PR is trying to solve. It’s also good to include:

    • who you paired with ex: “Paired with @daemonsy
    • link to the Trello card associated with the feature, bug, etc.
    • link to any past PRs this relates to
  • Before - unless included in ‘Whatsup,’ explain the situation in the current master. Include screenshots, gifs, links to code, etc.

  • After - explain how your PR has solved the problem, why you made certain decisions, etc. Include screenshots, gifs, links, or whatever else is going on to help reviewers see what you’ve done.

  • How to Review - list things you'd like to draw reviewers’ attention to, instructions for testing, things to consider when evaluating the PR, etc.

  • A label - we use labels to lump PRs into the functional area they fall into. If there’s no functional change we label it refactor. If it touches several functional areas, we add multiple tags and and expect signoff for each.

  • A Mention - we have different non-functional groups such as: UI/UX, Security, Architecture, etc. If the PR is relevant to any of these groups we call them out in the PR so they can review and weigh in. This helps maintain a level of quality across our apps.

Example PR

In this PR the change was pretty straightforward so only screenshots were included in the “Before” and “After” sections. In more complex PRs you’ll often see a few sentences explaining what was going on before the change, why it needs to be changed/fixed, and how the new solution fixes it.

Helpful Tools

  • Skitch - I’m sure everyone knows about Skitch but in case you somehoe don't, skitch allows you to easily take screenshots and annotate them. Additionally, it’s owned by evernote so if you’re an evernote user it’s G-R-E-A-T!

  • Quickcast - is a Mac app that allows you to easily do screen recordings up to 3 minutes in length. I use this when I want to include a gif of some new feature.

  • CloudConvert - after you take the short video with Quickcast you then need to convert it into a gif. CloudConvert makes this super easy.

Reviewers Guidelines

  • Actually think through the change; avoid just commenting on syntax.

  • Challenge everything complex - there is always a simpler way to achieve the same outcome.

  • Ensure that the code follows the spirit of our code guides.

  • It can be helpful to peer review big PRs together with the author so they can provide additional context.

  • If there is disagreement between yourself and the PR author, call in a third person for an opinion.

Outcome

As mentioned, having a set of guidelines for how we do PRs keeps them consistent and easy to review and understand. This in turn makes reviews go faster, meaning we can ship more quality code. It’s also great for junior developers who don’t have much experience with PRs and code reviews. If you don’t already have a uniform way to make pull requests and code reviews I highly recommend it.

Comments

comments powered by Disqus