I have yet to really find a culture of what I would consider good code reviewing practices. There seems to be plenty of bad code reviewing practices to call out but rarely good practices. The greatest benefit to a code review that I've seen is simply to keep everyone aware of new features, classes, and code coming into a system. Not as a means of detecting bugs (tests are for that), or improving the code quality (superficial issues aside, this needs to be done long before it reaches the point of creating a pull request).
Bad Code Review Practices
Robin Schroer in Traps to Avoid When Reviewing Code Changes has several of these practices to keep an eye out on.
Some good practices Schorer points out are creating checklists of common items, setting a [[pomodoro]] timer to dedicate time to the practice of review, checking for duplication of behaviors, and checking for places where a developer might misunderstand the underlying nature of existing models.
Looks Good to Me
Schroer abbreviates this as LGTM. The act of simply rubber stamping all code that passes through review in order to avoid conflict or digress from other duties. The counter to this, is to use the [[pomodoro]] technique to set asside a dedicated period of time to really look at the code.
Code review focuses on linting the code. This can be done automatically in most cases.
The unit tests can provide a great bit of insight into how the code under review is intended to be used in the system. Needless to say, questions of the quality of or exhaustiveness of the test cases are frequently passed over in code reviews.
Not Understanding the Reason for the Code
Many a time, we are asked to evaluate code without first knowing anything really about the feature or the use case that this code is intended to resolve. Evaluating tests can get some insight, but rarely enough.
- Schroer, Robin. Traps to Avoid When Reviewing Code Changes. Salumi's Blog. Retrieved 2021-03-09.