# Contributing

## Build and test

```bash
git clone https://github.com/bastienallain/nextjs-seo-prerender-cli
cd nextjs-seo-prerender-cli
cargo build --release
cargo test --locked
```

There is no setup step. `rust-toolchain.toml` pins the compiler and rustup installs it on the first
`cargo` command, `rustfmt` and `clippy` included.

**The suite runs entirely offline.** The integration tests spawn the real compiled binary against a
fixture HTTP server written on `std::net` and bound to port 0, so it takes whatever port is free
and two runs never collide. Every host it names that is not the fixture is a `.invalid` domain,
which cannot resolve by definition, and the unreachable-host cases point at a closed port. Nothing
about the HTTP layer is mocked, and nothing needs a network, an account or a token. If the tests
pass on your machine with the cable unplugged, they pass in CI.

## Gates

Three commands must pass before a pull request:

```bash
cargo fmt --all --check
cargo clippy --all-targets --locked -- -D warnings
cargo test --locked
```

CI replays exactly those on Linux, macOS and Windows.

## Dependencies

The list is short on purpose. The binary is small and starts fast, and that is a property of the
product rather than an accident: this thing runs in CI, and a tool that adds seconds to every build
gets moved to a nightly job and then forgotten. A new dependency needs a reason in the pull
request, and "it saves twenty lines" is usually not one.

## Adding or changing a check

This is what most contributions here are, and the rules below are what make the tool worth running.

**A check returns typed `Finding`s, never formatted text.** Checks produce
`Finding { kind, severity, message, detail }`; the report module is the only thing that turns data
into bytes. That separation is what stops the human output, `--plain` and `--json` from ever
disagreeing about what a run found.

**A check reads no command-line flag.** `--allow-dynamic`, `--quiet` and `--verbose` change how a
finding is rendered and weighed, not whether it is produced. A check that branches on a flag makes
the same site report two different sets of facts.

**Severity is decided in one place.** `effective_severity` and `summarize` are the single
definition of "blocking", and all three output modes go through them. Do not compute a verdict
anywhere else.

**A false positive costs more than an omission.** This runs in CI. A tool that fails a build it
should not have is a tool someone disables — and a disabled tool catches nothing at all, forever.
So our own uncertainty is never an error: an undetermined rendering mode is a warning, because it
is our limitation and not the site's fault. When a rule is hesitating between flagging and staying
quiet, it stays quiet.

**Negative cases are part of the change.** A new rule arrives with fixture routes that must *not*
trigger it, not only the ones that must. A thin static page, a legal notice, a healthy page that
happens to look unusual — those are the cases that decide whether the rule can be trusted, and
they belong in the suite next to the positive ones. A check with no fixture does not go in: the
fixture is the difference between a diagnostic and a guess.

Two standing constraints. Concurrency is capped at 6 in code because this audits a site rather than
load-testing it; do not raise it. And a fifth question has a bar to clear: **a real failure that is
invisible where people normally look** — not in the page source, not in the browser, not at build
time, not in a classic SEO crawler. "A crawler already reports this" is a reason to say no.

## Adding a `FindingKind`

The serde slug is a public contract. Machine consumers match on `kind`, so a slug is never renamed
and never reused; the `message` beside it is prose and free to improve at any time.

A new variant must be added to the table in the README. A test walks that table and the enum in
both directions and fails if either lists something the other does not, or if the count the README
announces stops being true. An exhaustive `match` in the same test means a new variant fails to
compile before it can ship undocumented.

## Commits

[Conventional Commits](https://www.conventionalcommits.org): `feat:`, `fix:`, `docs:`, `test:`,
`refactor:`, `chore:`. Write the subject about *why* the change exists, not which files moved — the
diff already lists the files. One behaviour per pull request, and if the output changes, paste the
before and after.

## Comments

English, sober, and about the *why*. A comment that restates what the line below already says
plainly is noise that has to be maintained. A comment explaining why a threshold is 60 and not 50,
why a check refuses to follow a redirect, or which bug a piece of defensiveness comes from is worth
keeping forever.