How I review code as a tech lead: lessons from production

I used to think code reviews were about catching bugs. After leading teams through scaling events and production incidents, I've learned that how I review code as a tech lead shapes how my team thinks about quality, grows as engineers, and ships reliable systems. Reviews are for quality, knowledge sharing and mentoring—and getting that balance right changes everything.

Understand the goal first — Read the ticket and description before the diff

The worst reviews I've given start with me scrolling straight into the changed files. I miss context, misunderstand intent, and end up asking questions the ticket already answered. Now I always start with the JIRA card, PR description, or design doc. I read what the author was trying to solve, what constraints they faced, and what they considered but didn't implement.

Last week, I reviewed a change to our payment retry logic. The diff looked aggressive—retrying failed charges up to 10 times. But the ticket explained we'd lost $40k to unretried failures last quarter, and the product team had signed off on the UX trade-off. Without reading the ticket first, I would have flagged it as risky. Instead, I understood the problem we were solving.

This approach also makes me a better reviewer because I catch real issues, not hypothetical ones. If the ticket says "optimize the dashboard query," I won't complain that it doesn't add caching—that's scope creep, not a review comment. If it says "add caching," I will.

One practical tip: if the PR description is empty or vague, ask the author to fill it in before you review. Vague PRs usually mean vague thinking. Making the author articulate their goal often surfaces issues they'll catch themselves.

Big things first — Design, correctness and security before naming and style

I've watched junior developers spend 30 minutes debating variable names while a concurrency bug goes unnoticed. As a tech lead, I prioritize what matters: does this work? Is it secure? Does it fit the system?

I split my reviews into three layers, reviewed in order:

1. Design and correctness: Does the approach make sense? Are edge cases handled? Will this cause problems in production?
2. Security and performance: Are secrets leaked? Can this be exploited? Will it scale?
3. Clarity and style: Is the code readable? Do names make sense? Does it follow our patterns?

Let me show you the difference. Here's a function I reviewed last month:

async function processPayment(amount: number, cardToken: string) {
  const response = await fetch('https://payment-api.example.com/charge', {
    method: 'POST',
    body: JSON.stringify({ amount, cardToken })
  });
  return response.json();
}

My first instinct was to comment: "Add error handling. Use fetch properly. Extract the URL to a constant." But I caught myself. The real issues were:

• The card token is being sent in the request body in plain HTTP POST—it might be logged by proxies or stored in server logs.
• No timeout or retry logic—the payment could hang indefinitely.
• No idempotency key—if the request retries, you charge twice.

Those are design problems, not style problems. I flagged those first. Only after the author fixed those did we discuss naming and structure.

Here's a better version after the security and correctness review:

async function processPayment(
  amount: number,
  cardToken: string,
  idempotencyKey: string,
  timeoutMs: number = 5000
) {
  const controller = new AbortController();
  const timeout = setTimeout(() => controller.abort(), timeoutMs);
  
  try {
    const response = await fetch(process.env.PAYMENT_API_URL, {
      method: 'POST',
      headers: {
        'Idempotency-Key': idempotencyKey,
        'Authorization': `Bearer ${process.env.PAYMENT_API_KEY}`
      },
      body: JSON.stringify({ amount, cardToken }),
      signal: controller.signal
    });
    
    if (!response.ok) {
      throw new Error(`Payment API returned ${response.status}`);
    }
    
    return response.json();
  } finally {
    clearTimeout(timeout);
  }
}

Now the review shifts to: is the error handling right? Should we retry? That's a productive conversation because the foundations are sound.

Ask, don't command — 'What happens if this fails?' teaches more than 'add error handling'

Commands block learning. Questions unlock it. I stopped writing "Add error handling here" and started writing "What happens if the database connection drops at line 42? Does the user get a response?"

The difference is profound. When I command, the engineer might add a try-catch and move on. When I ask, they think through the failure mode, consider the user impact, and own the solution. They learn something that applies to the next 50 functions they write.

Here's an example from a Redis cache layer I reviewed:

async function getUserProfile(userId: string) {
  const cached = await redisClient.get(`user:${userId}`);
  if (cached) return JSON.parse(cached);
  
  const profile = await db.query('SELECT * FROM users WHERE id = ?', [userId]);
  await redisClient.set(`user:${userId}`, JSON.stringify(profile), 'EX', 3600);
  return profile;
}

Instead of saying "Add error handling for Redis failures," I asked:

What happens if the redisClient.get() call times out? Does the request wait forever, or does it fall through to the database? Should it?"

That question made the author think about failure modes they hadn't considered. They decided to add a timeout, catching Redis timeouts separately and falling back to the database. Then they realized the same logic should apply to the set() call. One question sparked three improvements.

Asking also builds trust. Commands make people defensive. Questions invite collaboration. I've found that teams who get asked good questions grow faster and own their code more deeply.

Automate the trivial — Formatting and lint rules belong to tools, not reviewers

I don't comment on formatting, import order, or line length. ESLint comments are noise that buries real feedback. My team has linters, formatters, and pre-commit hooks. If something can be automatically checked or fixed, it shouldn't require a human reviewer.

This saves review time and keeps the conversation focused. When I review, my comments are about logic, not semicolons. The author knows that every comment I leave is either a learning opportunity or a blocking issue.

We use a standard setup across our projects:

• Prettier for formatting (configured once, never discussed again)
• ESLint for code quality with strict rules
• TypeScript strict mode to catch type bugs
• Pre-commit hooks to fail if linting or types don't pass

A PR that reaches my review has already passed linting. That means when I see code, I'm reviewing decisions, not syntax. It's a cleaner, more valuable review for everyone.

Approve with trust — Not every comment needs to block a merge

I distinguish between blocking comments and suggestions. Blocking comments are about correctness, security, performance, or maintainability—they stop the merge. Suggestions are improvements that can land in a follow-up or don't change the outcome.

Early in my career, I'd block merges over things like "this could be more elegant" or "we should add a helper function." Now I recognize that as scope creep. If the code is correct and the approach makes sense, I approve it. If I have a suggestion, I leave it as a comment and trust the author to decide if it's worth doing.

This matters for team velocity and morale. Engineers feel trusted. They also move faster because they're not waiting for perfection. I've learned that good enough today, with room to improve, ships more than perfect never.

I use a simple framework:

When I approve, I often leave suggestions as comments or invite the author to improve it in a follow-up PR. This keeps momentum high and makes reviews feel collaborative, not adversarial.

Common mistakes

Reviewing code without context: Diving straight into the diff without reading the ticket leads to questions the author already answered and misses the actual problem. Always start with the brief.

Nitpicking style over substance: Commenting on formatting, naming, and structure before you've verified the code works correctly and securely wastes everyone's time. Automate style and focus human reviews on thinking.

Being a gatekeeper instead of a mentor: If every review is a list of commands, your team won't grow. Questions, suggestions, and trust teach more than rejections. Your job is to raise engineers, not block PRs.

Blocking on preferences: "I wouldn't have written it that way" is not a reason to reject. If the code is correct and fits the system, different approaches are fine. Avoid imposing your style when the author's is equally valid.

Ignoring time pressure: Slow reviews block teammates. If a PR is correct but could be improved, approve it and file a follow-up. Don't make people wait days for a perfect review of a good PR.

Key takeaways

• Read the ticket and context before the diff. You can't review code if you don't know what it's supposed to do.
• Prioritize by impact: design and correctness first, then security and performance, then clarity and style.
• Ask questions instead of issuing commands. Questions teach, commands just obey.
• Automate what can be automated. Linters and formatters should enforce style, not reviewers.
• Distinguish between blocking issues and suggestions. Not every comment should stop a merge.
• Trust your team. Code doesn't have to be perfect to be good, and slow reviews hurt velocity and morale more than imperfect code hurts quality.
• Remember that reviews are about quality, knowledge sharing, and mentoring—not about gatekeeping or proving you're smart.