Effective Code Review
Code reviews can help you find issues before they hit production, as well as share knowledge with the rest of your team. There are a few things that I’ve found useful when doing them. Most of these fall on you as the author, but they’ll help you as a reviewer too.
Problem statement
Describe the problem your PR is trying to solve and what you want to see happen. Your reviewer should know why you’re making this PR. You can also include what impact this will have and a link to the story.
Mention any compromises you’ve made in your PR. This can prevent your reviewer from commenting on something that you had to do.
Size
I know that SLOC isn’t always the best way to measure the size of your PR, but I’ve found that between 50 and 200 lines of code is a good size, including your tests. If you can’t describe your PR in one sentence, it’s probably too big.
There will be some PRs that don’t fit this, like migrations or large refactors. You can always split your PR up into multiple PRs and number them. Stacking PRs can be useful for this. Just make sure that each PR only does one thing.
Testability
Make sure to include the commands that will allow someone to verify your work, along with any seed data or flags they need to run them.
If your change is visual, it’s nice to have a screenshot. If your change alters behavior, have some sort of steps that allow someone to verify it. If you have a preview link, that would also be nice to have in the description.
Rollback plan
Assume you’ll need to revert your change on the same day you ship it. This is easier when your diff only contains what you intended to change. Don’t add that little refactor you wanted to do to the code around your change.
Use feature flags or config toggles when applicable. Try to avoid adding migrations that can’t be reverted to a diff that’s changing behavior. If you must have a migration, make sure to mention it and how to revert it (or that there isn’t a way to).
Relevant reviewers
Add reviewers that own the code affected by your change, and don’t tag too many people. The more people you tag, the less likely someone will feel the need to review it. Someone who knows that area of the codebase is usually enough, plus maybe one generalist reviewer.
If you don’t know who can approve, state that in your description and someone can direct you to the right person.
Feedback
Use your comments on correctness and risk. Use static analysis tools like linters and formatters to find style and format issues so they’re never brought up in a PR.
If you have a nit that falls under readability or your team’s standards, bring it up once and document it so others don’t have to. You can also label your comments with a priority, e.g. blocker, suggestion, question or nit, so the author knows what has to change and what’s optional.
There’s no need to create a new workflow to do any of this. Just pick the one your team struggles with most and apply it to your next PR.