intermediate30 minby DevFox

Code Quality Tooling

Formatters and linters that end style arguments, pre-commit hooks that catch problems before review, and wiring the same checks into CI so they cannot be skipped.

Python
Tooling

The pull request has nineteen comments. Four are about the bug. The other fifteen are about blank lines, import order, a variable that should be named differently, and whether the dictionary should have a trailing comma. The reviewer spent their attention on whitespace and had little left for the logic.

Every one of those fifteen comments could have been made by a program, before the review, for free. By the end of this lesson your project will format itself, catch a real class of bug automatically, and run all of it in CI — so review is about whether the code is right rather than whether it is tidy.

Two different jobs

A formatter

Decides where the line breaks go.

It rewrites your code into one canonical layout and does not care in the slightest what the code does.

Its purpose is to end the conversation.

A linter

Points at constructs that look wrong.

An unused import, a mutable default argument, a bare except, a variable assigned and never read.

Some of what it finds is style. A useful share is a bug.

Two tools, deliberately kept apart — a formatter that argued about correctness would be dangerous, and a linter that reformatted would bury every real finding in a huge diff

Ruff

ruff does both, quickly enough that it runs on save:

bash

python -m pip install ruff

ruff format .              # rewrite files into the canonical layout
ruff check .               # report problems
ruff check --fix .         # fix the ones it can fix safely

Configure it in pyproject.toml, next to everything else:

toml

[tool.ruff]
line-length = 88
target-version = "py311"

[tool.ruff.lint]
select = [
    "E", "W",     # pycodestyle
    "F",          # pyflakes - undefined names, unused imports
    "I",          # import sorting
    "B",          # bugbear - real bug patterns
    "UP",         # pyupgrade - modern syntax
    "SIM",        # simplify
    "RUF",        # ruff's own rules
]
ignore = ["E501"]      # the formatter owns line length

B is the group worth enabling first, because it catches things this course has warned about:

python

def add(item, target=[]):        # B006 mutable default
    ...

try:
    risky()
except:                          # E722 bare except
    pass                         # S110 try-except-pass

for photo in photos:
    results.append(transform(photo))    # PERF401 use a comprehension

Those are not style opinions. The first is the trap from the functions lesson, and a tool finds it in every file in under a second.

Formatting is not worth discussing

Bad — a style guide people are asked to follow.

text

CONTRIBUTING.md
  - Use 4 spaces, never tabs
  - Break lines at 88 characters
  - One import per line, standard library first
  - Trailing commas in multi-line literals

Good — a formatter that applies it.

toml

[tool.ruff]
line-length = 88

bash

ruff format .

A written convention is enforced by humans reading diffs, which means it is enforced inconsistently, generates review comments nobody enjoys writing, and is violated most by whoever is busiest. Automating it removes the enforcement work entirely, and — the part people underrate — removes the decisions. Every minute spent deciding where to break a line is a minute not spent on the problem.

Which formatter you use matters far less than that you use one. The one legitimate rule is that it runs automatically, because a formatter people must remember to run produces diffs full of reformatting noise from whoever ran it last.

The type checker belongs here too

From the type hints lesson, now as part of the toolchain:

toml

[tool.mypy]
python_version = "3.11"
warn_unused_ignores = true
warn_redundant_casts = true

[[tool.mypy.overrides]]
module = "photo_tools.captions"
disallow_untyped_defs = true          # strict, module by module

The per-module override is the adoption strategy: green today, stricter each week, one module at a time.

Running it before the commit

Tools that run when someone remembers do not run. pre-commit attaches them to git commit:

yaml

# .pre-commit-config.yaml
repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    rev: v0.6.9
    hooks:
      - id: ruff
        args: [--fix]
      - id: ruff-format

  - repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.6.0
    hooks:
      - id: trailing-whitespace
      - id: end-of-file-fixer
      - id: check-yaml
      - id: check-added-large-files
      - id: detect-private-key

bash

python -m pip install pre-commit
pre-commit install                 # once per clone
pre-commit run --all-files         # the first time, to fix everything

Now the formatter runs on the files you are committing, and only those. detect-private-key and check-added-large-files are worth their place on their own — both prevent mistakes that are painful to undo once pushed, which is the secrets rule from the last lesson given teeth.

Keep hooks fast. A pre-commit hook that takes thirty seconds teaches people to use --no-verify, which is worse than not having it. The full test suite belongs in CI, not here.

And again in CI

Local hooks can be skipped. CI cannot:

yaml

name: checks
on: [push, pull_request]

jobs:
  check:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-python@v5
        with:
          python-version: "3.11"

      - run: pip install -e ".[dev]"
      - run: ruff format --check .        # fails; does not rewrite
      - run: ruff check .
      - run: mypy src/
      - run: pytest --cov=photo_tools

--check rather than a rewrite is the important difference. In CI you want a failure telling the author to run the formatter, not a bot committing to their branch.

Which gives the same checks three chances to catch something, each one slower and harder to ignore than the last:

On save
Instant, in the editor. Fixes most of it before you have finished the thought.
On commit
pre-commit, on the changed files only. Keep it under a few seconds or people learn --no-verify.
On push
CI, on everything, with --check. Skippable by nobody.

The same rules, three times — by the time a human reads the diff there is nothing mechanical left in it

Run each step separately so the log says which one failed. A single make lint that does all four gives you one red cross and a scroll.

What tools cannot do

Worth being clear about, so the tooling does not create false confidence.

They cannot tell you the code is correct — a linter is happy with a function that computes the wrong thing tidily. They cannot tell you a name is good, only that it is consistently formatted. They cannot judge whether an abstraction earns its place, whether an error message helps, or whether the tests assert on behaviour or implementation.

That is the point of automating the rest. Everything a machine can check should be checked by a machine, so human review spends its attention on the things only a human can judge — which are the things that were getting fifteen comments' worth less attention in the pull request at the top of this lesson.

A cheat sheet to keep

text

TWO JOBS
  formatter   canonical layout; ends the argument
  linter      suspicious constructs; some are real bugs

RUFF - both, fast
  ruff format .           rewrite
  ruff check .            report
  ruff check --fix .      fix what is safe

  select = ["E","W","F","I","B","UP","SIM","RUF"]
  ignore = ["E501"]       the formatter owns line length
  start with these; do not enable everything on day one

  B006 mutable default    E722 bare except
  F401 unused import      F821 undefined name

TYPE CHECKING
  mypy or pyright, in the same config file
  per-module overrides: strict where you have annotated

PRE-COMMIT
  pre-commit install            once per clone
  pre-commit run --all-files    the first time
  ruff, ruff-format, trailing-whitespace, check-yaml,
  detect-private-key, check-added-large-files
  keep it FAST - a slow hook teaches --no-verify

CI - because hooks can be skipped
  ruff format --check .    fail, do not rewrite
  ruff check .
  mypy src/
  pytest
  separate steps, so the log names the failure

SUPPRESSIONS
  narrow and explained: # noqa: B006  # type: ignore[arg-type]
  warn_unused_ignores finds the ones that are now stale

WHAT THEY CANNOT CHECK
  correctness, naming, whether an abstraction earns its place,
  whether a test asserts behaviour or implementation
  automate the rest so review has attention left for these

Where to go next

Your project now formats itself, catches a real class of bug before review, and enforces all of it where it cannot be skipped. The argument for this is not tidiness — it is that human attention is the scarcest thing in a review, and spending it on import order is a waste of the only thing that can catch a wrong abstraction.

Next is Profiling Before Optimising, the last lesson of this course. Tools that tell you what is wrong with your code have a counterpart: tools that tell you where the time actually goes, which is almost never where you guess.

Before you move on, add ruff to a project you already have and run ruff check . without fixing anything. Read what it finds before changing a line. In most codebases there is at least one genuine bug in that first report — usually an unused import hiding a refactor that was never finished, or a mutable default nobody had hit yet.