In my 20 years of experience, code reviews have been most successful when there have been defined purpose, with code review principles. Applying the following 3.1 rules as the purpose are the keys to the most useful code reviews. Do:
- find bugs
- improve quality
- share knowledge
Also, in my 20 years of experience, the following in code reviews have been enitrely counter-productive.
- strive for perfection (do not strive for perfection)
Principle 1: Find Bugs
There are bugs. Find them. The code has to work. The test are written and they pass. Read the ticket or story or requirments - or however you define what needs to be done.
When asked to review code, it's not just reviewing code. It's making sure the code works. Make sure your colleague has crafted code that the business expects, with outcomes as defined in the requirements.
Make your systems and environments trival for developers to check out the code and run it. Let them try to break it the ways they know how.
Same goes for the tests - are they actually testing anything? Do they actally test what they're supposed to? Read the tests. Care for the tests. Run the tests. Break the tests. Love your tests, and they will love you back when you most need them.
Anyway, wouldn't it be embaressing 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 introducted an issue?
Find the bugs, especially when there are hidden in beautifully crafted code.
When you find the bugs, there shouldn't be many, because your pull requests are small. But if there are many, triage them, determine what is must-fix for this piece of work, and what can we 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, 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 opportinities to share knowledge in the pull request
- be collaborative, positive, inclusive, and enjoy the shared custody of the code
Principle 3.1: Do not strive for perfection
If it is a particularly weak piece of work, don't bombard the pull request with 20 negative code review comments. Choose the most important things, make polite notes for feedback and, take it off-line.
Yes I know, taking it off-line seems so archaic, but sometimes talking and connecting as humans is better. At least consider it. 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, rather than "sending it back" to try again.