Code review isn't just about code

April 6, 2021

When you review a pull request, what do you look for?

Bugs? Misspelled words? Broken error handling? Missing unit tests?

That’s all fine and good. But there’s one aspect of pull request review that I think usually does not get enough attention: Metadata

I never metadata I didn’t like

The pull request title and description, as well as the individual commit messages (and each commit’s contents) are all important parts of a pull request, and deserve attention during a review.

To get you started thinking about this, here are some things I always look for when reviewing a PR:

  • Does the content of the PR match the title?
  • Does the PR description adequately explain the intention? (i.e. If you must ask for clarification, the description is probably lacking)
  • Does the PR address one concern (i.e. a single bug fix, a single refactor, or a single feature)
  • Does each commit message make sense in isolation? (i.e. “Bugfix” or “commit a missing file” don’t pass muster)

Related Content

"Readability" is subjective

Ask yourself: Will the least experienced developer likely to read this code be able to understand it?

Go Code Roast #2: readability.js port

In this video, I roast a port of a Mozilla Javascript library, [readability.js](https://github.com/mozilla/readability) to Go.

Go Code Roast

In this video, I roast some Go code! That is, I review it as if it were submitted as part of a job application screening. I talk about what I like, what I don't like, and how I would do things differently.