I end up reviewing a lot of code, and while doing code review (and getting reviews) used to take up a lot of time, I think I’ve gotten better at doing reviews, and knowing what’s important to comment on and what doesn’t
- The code review process is not there to discover bugs. Write tests to catch bugs, and use the code review process to learn about a specific change, and find things that are difficult to test for.
- As yourself if something is difficult to follow, and comment on that. If you can’t figure out what something is doing, or you have to read it more than once, then that’s probably a problem.
- Examine and comment on the naming of functions. Does the function appear to do what the name indicates.
- Think about the interface of a piece code:
- What’s exported or public?
- How many arguments do your functions take?
- Look for any kind of shared state between functions, particularly data that’s mutable or stored in globally accessable, or package local structures.
- Focus your time on the production-facing, public code, and less on things like tests and private/un-exported APIs. While tests are important, and it’s important that there’s good test coverage (authors should use coverage tooling to check this), and efficient test execution, beyond this high level aspect, you can keep reading?
- Put yourself in the shoes of someone who might need to debug this code and think about logging as well as error handling and reporting.
- Push yourself and others to be able to get very small pieces of code reviewed at a time. Shorter reviews are easier to process and while it’s annoying to break a logical component into a bunch of pieces, it’s definitely worth it.