Raimund Krämer

In the past, my advice for effective code reviews was something along the lines of:

• Use pull requests.
• Be kind, assume good intent.
• Don’t interrupt others, work asynchronously.
• If the whole PR is a mess, do multiple iterations (asynchronously, of course).
• … and some more.

Nowadays, after years working (more or less voluntarily) with pull requests, and sometimes without, this has changed quite a bit. My advice is now:

• Don’t use pull requests. Instead, actually work together like a team of professionals. It’s not a global open source project, or is it?
• If you’re not gonna review it and blindly approve, just decline the review and let someone else do it.
• Don’t start more work only to then switch back and forth between tasks.
• Don’t accept low quality just because you’re afraid of nitpicking, but also don’t block integration for cosmetic issues. Are we still currently iterating on this code anyway?
• If the whole changeset is a mess, mentor your colleague. Fix it together.
• If there are no tests it doesn’t work. Review the tests first. Also, see above: Work together instead of being surprised by a test-less pull request after the fact.
• If the design is wrong/bad, that’s discovered when the feature is finished, and it has been created by a single developer, that’s a pressing cultural issue to be fixed ASAP. If this happens consistently, this is likely a root cause of your budget problems.