What code review is for

One bullet, from a course with other things to teach

The opening lesson of "GitHub pull requests" gives code review one bullet out of four: "a second set of eyes catches bugs, suggests better approaches, and spreads knowledge of the codebase across the team". That course had seven lessons of GitHub mechanics to cover: the diff view, inline comments, approvals, branch protection and three merge strategies. It never had room to ask what makes a reviewer's comment worth reading, or what a reviewer should look for. That's this course's job.

Not a rerun of clicking things

This course doesn't repeat opening a PR, leaving an inline comment, resolving a conversation or picking a merge strategy. "GitHub pull requests" already covered all of that. This course assumes you can do it and asks a different question. You can leave any comment you want, so which comments are worth leaving, and how do you decide?

Google's engineering practices, the source this course leans on

Much of what follows comes from Google's published engineering-practices documentation, guidance that its engineers review code under. Its standard for when to approve a change reads: "reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn't perfect". Perfection isn't the bar. The same page says there is "no such thing as "perfect" code", only better code.

What this course covers

Lesson 2 looks at what CI catches in this app and what it leaves for a person, using the project's .github/workflows/ci.yml. Lesson 3 covers how serious a comment is, nit versus optional versus required, with the labels Google's docs define. Lesson 4 is about tone: comment on the code, not the person. Lesson 5 covers the reviewer's own job, turnaround time and which of three verdicts to pick, going past the mechanics in approvals-checks-and-branch-protection. Lesson 6 is about receiving feedback, going past "reply with your reasoning" in responding-to-review-feedback to tell disagreement from confusion. Lesson 7 puts it all together on one small diff.

Where this leaves you

By the end, "leave a good review" means more than "read the diff and say something". It means knowing what's worth flagging, how loudly to flag it, and how to say it without it landing as an attack. It also means deciding, once every comment is in, whether the change is ready.