In my 20 years of experience, code reviews in a development team have been most successful with a defined purpose with code review principles. As a companion to code quality principles. Apply the following 3.1 principles to review code better and improve code quality. One helps the other.
- find bugs
- improve quality
- Share knowledge
Also, in my 20 years of experience, the following in code reviews have been entirely counter-productive.
- Strive for perfection (do not strive for perfection)
Principle 1: Find Bugs
There are bugs. Find them. You might think code reviews are not for finding bugs, but you're wrong. Beautiful code also has to work. The tests are written, and they pass. Read the ticket, story, or requirements — or however, you define what needs to be done.
When asked to review code, it's not just reading code. It's making sure the code works. Ensure your colleague has crafted code that the business expects and the rest of the development team expects, with outcomes defined in the requirements.
Make your systems and environments trivial for developers to check out the code and run it. Let them try to break it the ways they know how.
The same goes for the tests — are they actually testing anything? Do they actually test what they're supposed to? Read the tests. Care for the tests. Run the tests. Break the tests. Love your tests; they will love you back when you most need them.
Wouldn't it be embarrassing if you approved some code that didn't work? And even worse, approved some code that didn't work and the tests passed? Made it to production and introduced an issue?
Find the bugs, especially when there are hidden in beautifully crafted code.
When you find them, there shouldn't be many bugs because your pull requests are small. But if there are many, triage them, determine what is a must-fix for this piece of work, and what we can live with. Be pragmatic.
Principle 2: Improve Quality
When improving quality, automate the low-hanging fruit. Don't waste your valuable time looking for the right indenting. Use a tool like Prettier to solve as much as possible. When you can't automate something, have a set of rules. When there are gaps in the rules, have a set of engineering values that people can depend on when rules are lacking; for example, "we prefer simple over clever".
Then, analyse the architecture, code patterns, maintainability, performance bottlenecks, and names of things, and improve the quality of the code in material ways that matter.
Principle 3: Share Knowledge
- learn something from the pull request
- look for opportunities to share knowledge in the pull request
- be collaborative, positive, and inclusive, and enjoy the shared custody of the code
Principle 3.1: Do not strive for perfection
Don't bombard the pull request with 20 negative code review comments if it is a particularly weak piece of work. Choose the most important things, make polite notes for feedback and take it offline.
Yes, taking it offline seems so archaic, but sometimes talking and connecting as humans is better. At least consider taking it offline. It can help build relationships and smooth over what could potentially be a negative experience otherwise. Send the requestor a DM, and have a call about it, then maybe pair up and help make the code (not the person, the code, don't make it personal) meet expectations.
Make it about moving the work forward with the next action, a step towards a more bulletproof code, bulletproof product, rather than "sending it back" to try again.