Review Tooling

Use tools (linters, type checkers, test coverage) to automate the mechanical checks so reviewers focus on logic.

Why

Code review time is scarce, and spending it on mechanical checks (style, formatting, type errors, missing tests) is a waste — these are exactly the things tools do better than humans. A linter catches the style violation in 0.1s; a human spends 30 seconds finding it and writing a comment, the author spends 5 minutes fixing it and replying, and the cycle repeats for the next violation. By automating these checks in CI, the PR cannot be opened until the code passes the linter, the type checker, and the test coverage threshold — the reviewer never sees style violations, type errors, or untested code, and their time is spent on the things only humans can do: logic, architecture, security, naming. This makes reviews faster (15 minutes instead of an hour), more valuable (the feedback is about the design, not the formatting), and less frustrating (nobody enjoys commenting on a missing semicolon).

When

Set up the tooling at project start and enforce it in CI. Add tools incrementally as the team identifies checks that are repetitive.

How

1. Linter (ESLint, Biome, Ruff, golangci-lint)
   - Enforce style, catch common bugs, forbid unsafe patterns
   - Run on every PR in CI, fail on violations
   - Configure rules to match the team's conventions
   - Auto-fix what can be auto-fixed (formatting, import order)

2. Formatter (Prettier, gofmt, rustfmt, black)
   - Eliminate formatting discussions — the tool decides
   - Run on save (editor) and in CI (enforce)
   - No formatting comments in review ever again

3. Type checker (TypeScript, mypy, pyright)
   - Catch type errors before runtime
   - Fail the build on type errors
   - Use strict mode (no implicit any, no null without explicit check)

4. Test coverage (jest --coverage, pytest --cov, go test -cover)
   - Require coverage for new code (e.g., 80% of new lines)
   - Fail the build if coverage drops below the threshold
   - Coverage is a floor, not a goal — 100% coverage does not mean good tests

5. Pre-commit hooks (husky, lefthook, pre-commit)
   - Run linter, formatter, and type checker on commit
   - Fail the commit if checks fail — the developer fixes before pushing
   - This catches issues before they reach CI, let alone review

Example

// .eslintrc.json — enforce style and catch bugs
{
  "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],
  "rules": {
    "no-unused-vars": "error",
    "no-implicit-coercion": "error",
    "eqeqeq": "error", // no ==, use ===
    "no-var": "error", // use let/const
    "prefer-const": "error"
  }
}

// .prettierrc — eliminate formatting discussions
{
  "semi": true,
  "singleQuote": true,
  "trailingComma": "all",
  "printWidth": 80
}

// tsconfig.json — strict type checking
{
  "compilerOptions": {
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true
  }
}

// package.json — coverage threshold
{
  "scripts": {
    "test": "jest --coverage --coverageThreshold='{"global":{"lines":80}}'"
  }
}

// .husky/pre-commit — run checks before commit
#!/bin/sh
npx lint-staged
# lint-staged config:
{
  "lint-staged": {
    "*.{ts,tsx}": ["eslint --fix", "prettier --write", "tsc --noEmit"]
  }
}

// .github/workflows/ci.yml — enforce in CI
name: CI
on: [pull_request]
jobs:
  checks:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - run: npm ci
      - run: npm run lint    # eslint
      - run: npm run format:check  # prettier --check
      - run: npm run typecheck  # tsc --noEmit
      - run: npm test  # jest with coverage

// Now the reviewer can focus on logic, not style:
// Reviewer comment: "This function has a race condition —
//   two requests can update the same record. Consider using
//   optimistic locking with a version field."
// (Not: "You used == instead of === on line 42.")

Common Problems

⚠Commenting on style in review when a linter could catch it — "use single quotes" is a linter rule, not a review comment, and the reviewer's time is wasted on a mechanical check.
⚠Setting up the linter but not enforcing it in CI — the linter passes locally, but a developer disables it (eslint-disable) or does not run it, and the violation ships.
⚠Requiring 100% test coverage — the team writes trivial tests (expect(true).toBe(true)) to hit the threshold, the coverage is 100% but the tests are worthless, and the metric becomes a game.
⚠Running all checks in CI but not locally — the developer pushes, waits 10 minutes for CI, discovers a lint error, fixes it, pushes again, and the cycle repeats 5 times before the PR is green.

Best Practices

✓Automate all mechanical checks (lint, format, typecheck, coverage) in CI and pre-commit hooks, so the reviewer never sees them — review time is spent on logic, architecture, and security, not style.
✓Run the checks locally on save (editor integration) and on commit (pre-commit hooks), not just in CI — the developer gets instant feedback and does not wait 10 minutes for CI to find a lint error.
✓Set a coverage threshold for new code (80% of new lines) rather than 100% of all code — this encourages testing new code without incentivizing trivial tests on old code.
✓Use strict TypeScript (strict: true, noImplicitAny, strictNullChecks) — the type checker catches entire categories of bugs (null dereference, type confusion) that reviewers would otherwise have to check manually.