There are a million checklists and blog posts out there on what to look for and how to give constructive feedback.
The thing I don't see discussed enough is how to appropriately scope a code review. When I'm reviewing commits, what should I leave out?
Here are some things I believe are out of scope for code reviews:
- Hypothetical business requirements
"What if the business wants X as well?" Then let them ask for it. That's what the product backlog is for.- Broad architectural debates
"Should we change the way that we're doing X in general?" Maybe we should. Let's put an item in the backlog to find a new approach. If we adopt a new pattern for similar functionality, we can revisit this code and refactor.- Fear of change
The perspective that the code under review must represent the last word on a particular topic. If the code under review is high quality, well-tested, and meets a known requirement, then it represents no danger to master. There's no shame in changing or even deleting the code in question with a future pull request if requirements shift.As with so many aspects of software engineering, setting the appropriate scope is key. For code reviews, stay focused.
