AR8: Engineering standards and code review
Feedback that helps
🎯You'll be able to
Write review comments that are kind, specific and actionable, with a severity, a suggestion, praise where it's due and a clear decision.
🏠The everyday picture
A good cricket coach says "your front foot is landing outside off stump; try a shorter stride", not "that was terrible". The player knows what to change and still wants to come back tomorrow.
📘What it really is
- Comment on the code, not the person: "this call isn't awaited", not "you forgot again".
- Be specific: the file, the line, what's wrong and why it matters.
- Suggest a fix, or ask a question if you're not sure.
-
Label severity: blocker (must change before merging: customers, money or data at risk), major, minor, nit (a preference), and praise.
-
Say what's good. Praise for a clear summary or a test teaches as much as criticism.
- End with a decision: Approve, Request changes (when there are blockers) or Comment.
- Avoid words that sting or talk down: "obviously", "just", "lazy", "you always".
📝A worked example
Input: line 10 of refunds.js, gateway.refund({ orderId, amount, token: REFUND_TOKEN });.
Steps:
- Poor: "Obviously wrong, you just forgot await."
- Better: "Severity: blocker.
gateway.refundreturns a promise that isn't awaited, so a failed refund is never seen and the function returns before the refund happens." - Suggestion: "Add
awaitand let the error reach the caller, or return a clear failure." - Praise elsewhere: "Good to see a test arrive with the feature."
Expected output: a findings table with file, line, severity, rule, comment and suggestion, and a decision of Request changes.
🌍Where it's used
- Teams that use labels such as "nit:" or "blocker:" so authors know what must change.
- Mentoring: review comments are one of the main ways junior engineers learn.
- Open-source projects with codes of conduct for reviews.
📝Case scenario
A new engineer's first three pull requests got blunt one-word comments ("No.", "Wrong."). They stopped asking questions and started hiding unfinished work. After the team adopted labelled, explained comments with praise where due, the same engineer's pull requests got smaller and better, and they began reviewing others.
🖱️Try it
Rewrite three harsh comments ("this is a mess", "why would you do this", "just use the logger") as kind, specific comments with a suggestion. Success: each names the problem, why it matters and what to do.
⚠️Common mistakes
- Comments about the person. Sign: "you always…". Fix: comment on the code.
- No suggestion. Sign: "this is wrong" and nothing more. Fix: suggest a fix or ask a question.
- No decision. Sign: twenty comments and the author doesn't know if they can merge. Fix: Approve, Request changes or Comment.
🛠️For your project
Project 3's tests 8 and 9 read reviews/pr-41.md: the blockers found, suggestions, at least one praise, no harsh words, and the
decision "Request changes".
✅Quick check
What makes a finding a blocker?
Show the answer
Merging it would put customers, money or data at risk, so it must change first.
Why include praise in a review?
Show the answer
It shows what to keep doing, and keeps people willing to ask for review.