Code reviews: principles
This course covers the judgment behind code review: which comments are worth leaving and how to decide. It draws on Google's engineering-practices documentation, whose standard is to approve a change once it improves overall code health, even if it isn't perfect. Lessons look at what this app's CI checks and what it leaves for a person, how to label comments as nit, optional or required, and how to keep the tone on the code. They also cover turnaround, picking a verdict, receiving feedback, and a comparison of two reviews of one diff.
What you'll learn
- ✓What this app's CI checks and what it leaves for a human reviewer
- ✓Labeling comments as nit, optional or required
- ✓Writing comments about the code instead of the person
- ✓Choosing between Approve, Request changes and Comment, and responding within one business day
- ✓Telling disagreement from confusion when receiving feedback
- ✓Comparing a rubber-stamp approval with a calibrated review
What code review is for
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,…
Read the full lesson free →Curriculum
7 lessonsWhat code review is for
Explains which comments are worth leaving, using Google's standard of approving changes that improve overall code health.
What CI already checks, and what it leaves for a human
Shows that this app's CI runs Brakeman, bundler-audit, importmap audit and Rubocop, but runs no tests.
Calibrating severity: nit, optional and required
Defines the Nit and Optional labels from Google's docs and why unlabeled comments read as required.
Tone: address the code, never the person
Shows why comments should address the code instead of the author, and treats teaching as part of review.
The reviewer's own job: turnaround and picking a verdict
Covers choosing among Approve, Request changes and Comment, a one-business-day response, and the cost of rubber-stamping.
Receiving feedback well: the other half of review
Separates disagreement from confusion on the author's side and suggests moving stuck threads to a quick call.
Putting it together: the same diff, two reviews
Compares a bare LGTM with a review that asks for an empty-list test and labels a nit.