Read this page once when you first join the project, read it again when you submit your first PR, and read it again when you receive your first request to review code.
Why Code Reviews are Important:
When performed with diligence and effort, code reviews have been proven to be the single best way to eliminate defects in production code.
Step-by-step guide
- When you create your PR, you will assign someone to review it, based on who you think will do the best job.
- Don't assign more than 2 people to your code review, because then we get the 'bystander effect' when there are lots of reviewers.
- When you receive a code review request, show the courtesy of responding within a day on when you can complete the code review.
- We should plan to submit PRs on Wednesdays to give reviewers time to get the reviews done by Friday.
- You can't send people thousands of lines of code, limit your pull requests to approx. under 500 lines. This does depend on the language involved. This is a heuristic, not a hard and fast rule.
- Once you have an idea of how the code in a PR will look, you should immediately begin thinking about how to break it up for the reviewer.
It's also a good time to reach out to a reviewer to let them know we'll be sending something their way and talk over how we're going to structure the set of PRs. - One way is to break it up by dependency: we issue a PR for new code with no dependencies, then a PR for code that depends on those, then a PR for stuff that depends on that, etc. Once all the new code is in, then we make the switch from depending on old code to new.
- While it feels like a hassle in the middle of coding to stop and do something else, it will actually take less time overall than to dump a large pull request on a reviewer---at least, if we want any quality in our code reviews.
- Once you have an idea of how the code in a PR will look, you should immediately begin thinking about how to break it up for the reviewer.
- You need to review every line of code. You should be able to understand the code.
- Provide feedback! Is the code expressive and easy to understand?
- Don't let egos get involved. Having a lot of comments is a good thing, this isn't about someone's code being criticized. It's about improving code readability, quality and finding bugs.
- In general, try to get your code reviewed every 48 hours.
- It's okay to have several code reviews in a sprint.
- It's okay to have several code reviews for a single ticket.
- If you are writing a lot of boilerplate code, you could drop more code in a single PR for review if desired, because the reviewer could learn the syntactic pattern.
- Don't hold on to your code until the issue is resolved, there is no need to do that.
- You can use Pseudo code and include your design, the code doesn't have to work. Comment it out if needed, because we don't want to break tests either.
- This is NOT an approval process, this is about having a conversation about how you should proceed forward, and is the code of high quality.
- It is harder to review code than it is to write it. Please write more comments and description outlining your work. Reviews take more effort - please write a paragraph about your code.
- Also if the person that has the context can't do your code review for some reason, the other person will have that much more context when performing the review.
Examples of some completed Code Reviews:
Related articles