How to conduct code reviews that actually improve architecture, mentorship, and team velocity — not just catch typos.
Code ReviewBest Practices
If your review comments could have been written by a linter, let the linter write them. Human attention is expensive and should go where tooling cannot reach: boundaries, naming, failure modes, and intent.
Automate the boring layer first
yaml
name: checks
on: pull_request
jobs:
verify:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: oven-sh/setup-bun@v2
- run: bun install --frozen-lockfile
- run: bun run lint
- run: bunx tsc --noEmit
- run: bunx vitest run --coverage
Review the seams, not the lines
This function passes review at line level and fails at design level: it mixes fetching, validation, and persistence, so nothing can be tested in isolation.
ts
// before — three responsibilities, untestable without a network
async function importUsers(url: string) {
const rows = await (await fetch(url)).json();
for (const r of rows) {
if (!r.email?.includes("@")) continue;
await db.insert(users).values({ email: r.email, name: r.name ?? "unknown" });
}
}
// after — a pure core with thin edges
export const parseUsers = (rows: unknown[]) =>
rows.map((r) => UserRow.safeParse(r)).filter((r) => r.success).map((r) => r.data);
export async function importUsers(url: string, fetchJson = defaultFetchJson) {
const valid = parseUsers(await fetchJson(url));
if (valid.length) await db.insert(users).values(valid);
return { imported: valid.length };
}
Comment conventions that reduce friction
–Label severity: 'blocking:', 'suggestion:', 'nit:'. Ambiguity is what makes reviews slow.
–Ask questions when you are unsure of intent; assert only when you are sure of a defect.