I’ve long been frustrated by the task of code reviews. Often, the pull request (PR) I’m reviewing involves a part of the codebase I’m not intimately familiar with. I read it, not quite understanding it, looking to see if I can offer some sort of useful feedback, and typically that feedback would be on the micro level (e.g., “you can simplify this function by calling this other library function instead”).
I recently started experimenting with a new review approach that I’m going to call top down code review. Here’s how it works: I start by understanding the code well enough so that I can write my own version of the pull request message, describing the PR in my own words. After I’ve done this, then I provide feedback.
I call this approach “top down” because the review that I end up generating starts with a “top down” description of the PR: the problem it’s trying to solve, and the solution approach, before diving into describing notable implementation details. Here are the reviews I’ve done in this style so far:
- https://github.com/spinnaker/keel/pull/1681#pullrequestreview-543335454
- https://github.com/spinnaker/keel/pull/1674#pullrequestreview-537791392
- https://github.com/spinnaker/keel/pull/1673#pullrequestreview-536820748
- https://github.com/spinnaker/keel/pull/1657#pullrequestreview-534881241
I’ve been finding this approach useful because it forces me to come to terms with how well I really understand the PR. If I can’t explain the PR in my own words, then I don’t really understand it. It also helps me figure out what questions to ask the original author to help clarify things for me.
I also get more of a sense of closure after doing the review. Even if I had no feedback to give, I understand the changes in a way that I didn’t before.
It’s an interesting idea and I like that the submitter could use it to verify the understanding of the person doing the review (and creating useful artefacts).
I do wonder whether this kind of summary belongs as part of the PR though, that than the approval?