Pull-Requests: Best Practices and Lessons Learned

Assume good faith

Particularly if you don't have a lot of experience yet, it may seem that your peers are being overly annoying or purposefully unhelpful when reviewing your PRs.

There do exist some people who do want to make others look bad on purpose, but it is much more often the case that reviewers just lack social skills or maybe just didn't have the time to write out a more nicely worded comment, which came off as rude.

Always assume your peers are honestly trying to maintain the quality of the codebase unless you have good reason to think otherwise.

Not just what/how, but why

The what and the how of some piece of code are usually easier to infer from the code itself.

The why, however, is where decisions carry more weight.

It is possible that a PR contains flawless code that works are expected but it just solves the wrong problem.

This is the type of thing more senior reviewers can help you with. But they need more context on the whys of your solution to help you with that.

Multiple small PRs

It's easier to review 2 PRs of 50 lines each than 1 PR with 100 lines of code.

You should test

It's not the reviewer's responsibility to test that the code works. You should do that.

By default, all code in a PR should already be working.¹

Reviewers should at most indicate which parts of the code lack tests.

Review your own PR first

Before asking people to review your PR, you yourself should go through the PR as if you were reviewing it.

Oftentimes we catch silly mistakes/typos just by looking at all changes we've made, all in one place.

Don't just automatically agree

You don't need to automatically subscribe to and accept every suggestion made.

Needless to say, you shouldn't disagree with everything by default, either.

Value your peers' time

Write a good PR description that details what you are trying to achieve and most importantly, why you are doing it (what problems or business needs drove you to it?)

Don't ever take for granted suggestions and feedback on PRs. At the very least, acknowledge every comment/suggestion with an emoji or reply.

Consistent quality will earn you trust

If you consistently write good quality code and learn from feedback, you will eventually be in a position where people will more or less trust you by default.

This is a double-edged sword though; you will be able to get your changes faster into production but it will force you to be much more careful with what you write.

Return the favour

Make a habit of reviewing other people's work too. Not only does this increase your team output but it also helps the team bond.

This is a high-leverage activity (meaning you have more impact with constant effort) but it is also a muscle that you can train and get good at.

1:PRs are not the place to verify immediate correctness, but rather architectural, style issues and edge cases, in addition to suggestions on how to better achieve goals.