luvv to help

Menu

Topic 1 of 7

Code Review Practices

Learn Code Review Practices for free with explanations, exercises, and a quick test (for Backend Engineer).

Published: January 20, 2026 | Updated: January 20, 2026

Table of Contents

Why this matters

Backend systems power critical user experiences. Code reviews are where bugs are caught, performance is tuned, security risks are reduced, and team knowledge is shared. As a Backend Engineer, you will regularly review pull requests (PRs), suggest improvements, and approve changes. Good reviews keep services reliable and teams productive.

Real tasks this skill supports
  • Evaluating API changes for backward compatibility.
  • Spotting performance bottlenecks (e.g., N+1 queries, missing indexes, inefficient loops).
  • Checking reliability: retries, timeouts, idempotency, error handling, observability.
  • Protecting security: auth, input validation, secret handling, least privilege.
  • Ensuring tests and docs match behavior and edge cases.
  • Maintaining consistency with team style and architecture.

Concept explained simply

Code review is a lightweight technical discussion on a code change. The goal is to improve the change and share context, not to judge the author. Keep it respectful, specific, and actionable.

Mental model: C.A.R.E.S.

  • Change understanding: What problem is solved? Is the scope focused?
  • Architecture & API: Does it fit the design and remain backward compatible?
  • Risks & Reliability: Errors handled? Timeouts, retries, idempotency, concurrency?
  • Efficiency: Reasonable complexity, memory, I/O, and DB usage?
  • Style, tests, docs: Readable, consistent, tested, and documented?
What is in scope vs. out of scope
  • In scope: correctness, security, reliability, performance, readability, tests, docs, maintainability.
  • Out of scope: personal preference unrelated to team standards, unrelated refactors, debating product scope beyond the PR.

Levels of feedback

  • Nit: Purely cosmetic; do not block.
  • Suggestion: Better alternative; non-blocking.
  • Required: Must change to meet standards or correctness.
  • Blocker: Approving would introduce risk; request changes.

Prefer approving with clearly labeled follow-ups for small, safe items. Ask for splitting oversized PRs.

Worked examples (3)

Example 1 — Missing timeouts and retries on external HTTP call

Diff (simplified):

+ func FetchRates(ctx context.Context) (Rates, error) {
+   resp, err := http.Get("https://api.exchangerates.example/rates")
+   if err != nil { return Rates{}, err }
+   defer resp.Body.Close()
+   // parse JSON...
+   return rates, nil
+ }

Good review comment:

  • Required: Use an HTTP client with timeouts; avoid default client without context.
  • Suggestion: Add retry with backoff for transient 5xx.
  • Required: Check non-200 status codes and return typed errors.
  • Suggestion: Add metrics and structured logs for failures.

Better code sketch:

client := &http.Client{ Timeout: 3 * time.Second }
req, _ := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
resp, err := client.Do(req)
// handle status, parse, metrics...
Example 2 — Potential N+1 query and missing index

Diff (simplified):

// For each order, fetch its items
for _, o := range orders {
  items := db.Query("SELECT * FROM order_items WHERE order_id = ?", o.ID)
  o.Items = items
}

Good review comment:

  • Required: This causes N+1 queries. Use a single query with IN (...) and group in memory.
  • Suggestion: Ensure index on order_items(order_id).
  • Suggestion: Add test for performance using sample dataset.
Example 3 — Logging sensitive data

Diff (simplified):

log.Printf("login failed for user=%s password=%s", user, password)

Good review comment:

  • Blocker: Do not log credentials. Remove password; consider hashing or redaction for user identifiers.
  • Required: Use structured logging with levels and fields.
  • Suggestion: Add a lint/check to prevent future leaks.

How to run a solid review (steps)

  1. Read the PR title and description. What problem is solved? Any scope creep?
  2. Skim the overall diff. Identify risky areas (auth, DB, concurrency, external calls).
  3. Run tests locally or in your head. Consider edge cases and failure modes.
  4. Deep dive with C.A.R.E.S. Check tests, docs, and configs.
  5. Write comments: specific, kind, and actionable. Label nit/suggestion/required.
  6. Summarize at the end: overall confidence, key risks, and whether approval is blocked.
  7. If large: request splitting by logical commits or feature flags.
Helpful comment templates
  • Question: "Could you share the reasoning for choosing X over Y?"
  • Suggestion: "Suggestion: extract this loop into helper to avoid duplication with Foo()."
  • Required: "Required: add timeout on this network call; otherwise it can hang under outage."
  • Nit: "Nit: rename to userCount to match existing naming."

Reviewer checklist

  • I understand the intent and scope of this PR.
  • Correctness: happy path and edge cases look right.
  • Reliability: errors, retries, timeouts, idempotency, concurrency.
  • Security: authZ/authN, input validation, secrets not logged.
  • Performance: no obvious hotspots (N+1, unnecessary copies, blocking I/O on hot path).
  • Tests: cover key paths and regressions; pass locally/CI.
  • Observability: logs, metrics, alerts where needed.
  • Style/consistency: follows team conventions and docs updated.
  • Scope: small enough; otherwise request split.

Common mistakes and how to self-check

  • Only commenting on style. Self-check: did I examine correctness, security, and performance?
  • Vague feedback. Self-check: can the author act on my comment without guessing?
  • Blocking on personal preference. Self-check: is this backed by team standards or risk?
  • Ignoring tests. Self-check: did I read or run tests and see what they actually verify?
  • Approving oversized PRs. Self-check: did I ask for smaller changes when needed?
  • Harsh tone. Self-check: would I be comfortable receiving this comment?
Rewriting harsh comments
  • Harsh: "This is wrong." Better: "I think this introduces a race condition. Could we guard with a mutex or use atomic here?"
  • Harsh: "Use better names." Better: "Suggestion: rename req to paymentReq to match other handlers and improve clarity."

Exercises

These mirror the tasks below. Do them now, then check solutions.

Exercise 1: Review a risky diff

See Exercise 1 in the Exercises panel. Write your review comments covering C.A.R.E.S. Label nit/suggestion/required.

Exercise 2: Rewrite comments to be kind and actionable

See Exercise 2 in the Exercises panel. Keep the technical point; improve tone and clarity.

  • I posted at least one blocker with justification.
  • I posted at least two suggestions with examples.
  • I rewrote harsh comments into respectful, actionable ones.

Mini challenge

Pick a recent PR from your own or sample code and review it using the C.A.R.E.S. checklist. Limit yourself to 20 minutes. Write a 3-line summary: main risk, main improvement, and approval status.

Practical projects

  • Create a team code review guide: include labels (nit/suggestion/required), response-time expectations, and examples of good comments.
  • Define a PR template with sections: context, approach, risks, test plan, screenshots or logs.
  • Set up automated checks (lint, tests, security scan) to reduce nitpicks.
  • Run a review dojo: take one historical incident-causing PR and practice reviewing it together.

Learning path

  • Before: Git fundamentals, project build/test commands, basic language syntax, team style guide.
  • Now: Code Review Practices (this lesson).
  • Next: Incident prevention through tests and CI quality gates, writing maintainable APIs, documenting architectural decisions.

Who this is for

Backend Engineers, SREs, and Platform Engineers who review or submit PRs and want consistent, high-signal feedback.

Prerequisites

  • Comfort with your stack (language, framework, DB basics).
  • Ability to run and read unit/integration tests.
  • Familiarity with Git and pull requests.

Next steps

  • Use the checklist on your next PR review.
  • Adopt the comment templates and keep them handy.
  • Take the Quick Test below. Everyone can take it for free; sign in to save your progress.

Practice Exercises

2 exercises to complete

Instructions

Read the diff and write review comments. Label each as Nit, Suggestion, Required, or Blocker.

// handler.go
+ func CreateUser(w http.ResponseWriter, r *http.Request) {
+   body, _ := io.ReadAll(r.Body)
+   var req map[string]string
+   json.Unmarshal(body, &req)
+   // TODO: validate
+   password := req["password"]
+   // hash: temporarily store plain for audit
+   log.Printf("creating user=%s password=%s", req["email"], password)
+   hash := sha1.Sum([]byte(password))
+   _, err := db.Exec("INSERT INTO users(email, password_hash) VALUES('"+req["email"]+"', '"+fmt.Sprintf("%x", hash)+"')")
+   if err != nil { http.Error(w, err.Error(), 500); return }
+   http.Redirect(w, r, "/welcome", 302)
+ }

Consider: input validation, security, SQL injection, error handling, logging, HTTP semantics, hashing, and tests.

Expected Output
A list of labeled comments addressing security (do not log secrets), hashing (use bcrypt/argon2), SQL injection (use parameterized queries), validation and error handling, correct HTTP status codes, and adding tests.

Code Review Practices — Quick Test

Test your knowledge with 8 questions. Pass with 70% or higher.

8 questions70% to pass
Writing Technical Specs

Have questions about Code Review Practices?

AI Assistant

Ask questions about this tool