How to make a good code review
A collection of principles that I found interesting to make an effective code review for both reviewers and authors.
Focus on Functional Requirements First
"If you don’t get the requirements right, it does not matter how well you do anything else" - Karl Wiegers
The code should be reviewed in the following priorities
Functional requirements: the PR should cover all items in the functional requirements including unit tests and coverage
Clean and maintainable: align with coding standards
Optimal
The order of the priorities is important. Focusing on clean and maintainable or optimized code first might cause distraction and missing functional requirements.
Pull and Test the PR
Pull and test the PR on your local and/or testing environment to make sure the code works.
Try to break the code https://blog.codinghorror.com/doing-terrible-things-to-your-code/
It's not always obvious if the PR has already fulfilled all of the acceptance criteria or contains any side effects if we don't pull and test it.
Make a thorough review
Don't review just a portion of the PR then after the PR is updated, make another suggestion for other parts.
Timely response
Don't propose the change and forget. Try to follow up with the PR and approve when the PR is updated.
Back and forth kills productivity
Reserve dedicated time. Pair or all-hands review to avoid back and forth which kills productivity.
Prefer Progression over Perfection
Focus more on core issues, less nitpick.
Use simple, deference, and actionable language
"Software Design is an Exercise in Human Relationships" - Kent Beck
Suggest the desired changes precisely.
Open to learn
"The aim of argument, or of discussion, should not be victory, but progress" - J. Joubert
"It is not who is right, but what is right, that is of importance" - Thomas Huxley
Every feedback is a chance to learn and to make the PR better. Try to incorporate it into the PR where possible.
Prepare and embrace the change
"The only constant in life is change" - Heraclitus
Changes should be managed and kept minimal after the sprint starts to make the team more productive. However, changes happen sometimes. So let's prepare and embrace it.
Understand the requirements, and make a POC implementation to ensure the approach is feasible.
Create a draft PR and do a pair/all-hands review to get early feedback.
Create a new backlog ticket if the change cannot be accommodated in the current sprint.
References
https://www.developer.com/guides/effective-code-reviews-without-the-pain/
https://blog.codinghorror.com/the-ten-commandments-of-egoless-programming/
https://blog.codinghorror.com/doing-terrible-things-to-your-code/
https://google.github.io/eng-practices/review/reviewer/standard.html
https://dev.to/codemouse92/10-principles-of-a-good-code-review-2eg
https://www.monterail.com/blog/2016/egoless-programming-code-review