Apply code review and quality practices, including peer review, style guides, linters and static analysis

What this dot point is asking

NESA wants you to describe how code review works in a software team, the mechanical tools that support it, and how the combination of human review and automated tools produces high-quality code.

The answer

Code review

A peer reviews every change before it is merged. In modern Git workflows, this happens on pull requests:

1. The author opens a PR.
2. Reviewers read the diff, leave inline comments, ask questions, request changes.
3. The author replies or revises. Comments are resolved.
4. When reviewers approve and automated checks pass, the PR can be merged.

What reviewers look for

• Correctness: does the code do what the PR says it does?
• Tests: are there tests for the change? Do they cover the edge cases?
• Security: are inputs validated, outputs encoded, queries parameterised?
• Maintainability: is the code clear? Are the names good? Is the structure consistent with the rest of the codebase?
• Design: is this the right approach? Are there simpler alternatives?
• Documentation: are docs updated? Are non-obvious decisions noted?
• Performance: any obvious hot spots? N+1 queries? Memory leaks?

Constructive review

Best practice from industry:

• Comment on the code, not the person. "This function could be clearer", not "you wrote unclear code".
• Distinguish blocking issues from suggestions. Use prefixes like [blocking] or [nit].
• Ask questions rather than make pronouncements. "Why this approach?" surfaces assumptions.
• Approve when ready. Holding approval for minor preferences slows everything down.
• Authors: take feedback seriously, push back when warranted, do not take it personally.

Style guides

A document (or shared linter config) describing how the team writes code:

• Naming conventions (snake_case for Python variables, camelCase for JavaScript).
• File and folder structure.
• Comment style.
• Error handling patterns.
• Import order.

Examples: PEP 8 for Python, Airbnb JavaScript Style Guide, Google's various style guides.

Linters and formatters

Automated tools that enforce style and catch common mistakes:

Run automatically in CI on every PR. Failed lint blocks the merge.

# A ruff configuration in pyproject.toml
[tool.ruff]
line-length = 100
target-version = "py311"

[tool.ruff.lint]
select = ["E", "F", "B", "I", "S", "C90"]
# E = pycodestyle errors
# F = pyflakes (unused imports, undefined names)
# B = bugbear (common bug patterns)
# I = import order
# S = security (bandit)
# C90 = complexity (McCabe)

Static analysis

Goes deeper than linting. Analyses the code structure without running it.

• Type checkers: mypy (Python), TypeScript, Flow. Catch type mismatches and missing null checks.
• Security scanners: Semgrep, SonarQube, Snyk Code. Match common vulnerability patterns.
• Complexity analysers: McCabe complexity, cyclomatic complexity, identify functions that should be refactored.
• Dead code detectors: identify unreachable code and unused imports.

Run in CI alongside tests.

Pre-commit hooks

Run linters and formatters automatically on every commit, before the change leaves the developer's machine. Saves a round trip with CI.

# .pre-commit-config.yaml
repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.5.0
    hooks:
      - id: ruff
      - id: ruff-format
  - repo: https://github.com/pre-commit/mirrors-mypy
    rev: v1.10.0
    hooks:
      - id: mypy

Architecture decision records (ADRs)

For decisions that shape the codebase, write a short record:

# ADR 0007: Use PostgreSQL not MongoDB

## Status
Accepted, 2026-05-15

## Context
We need a database. The team has more SQL experience than NoSQL.

## Decision
Use PostgreSQL 15.

## Consequences
- Strong consistency by default.
- Mature tooling (psql, pgAdmin).
- The team must design relational schemas, including for semi-structured
  data (use jsonb columns where needed).

ADRs preserve the why behind technical choices, so future maintainers can revisit them with full context.

Metrics

Some teams track:

• Time to review: median hours from PR open to first review.
• Time to merge: median hours from PR open to merge.
• Review thoroughness: comments per 100 lines of diff.
• Coverage: test coverage percentage.
• Lint debt: open lint errors.

Use metrics as signals to investigate, not targets to optimise (Goodhart's law).