fmt tooling is incomplete: fmt-check does not verify goimports, and no formatter covers Markdown #119
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two related gaps in the formatting entrypoints. Both mean
make fmt-checkcan pass on a tree thatmake fmtwould change — which is exactly what a format gate exists to prevent.1.
script/fmt-checkdoes not check whatscript/fmtappliesscript/fmtruns two tools:script/fmt-checkchecks only the first:goimportsis never verified. So a file with mis-grouped or mis-ordered imports — somethingmake fmtwould rewrite — sails throughmake fmt-check, and therefore throughmake check, CI, and the Docker build. The gate is asymmetric with the formatter it is supposed to police.This is the same shape of defect as #115 (
script/cibuildreporting a green it did not earn): a check that appears authoritative while silently not covering part of what it claims.Note
gofmt -l .andgoimportsboth walk the tree from the repo root — confirm whatever you add does not descend into vendored or generated paths if any are later introduced.2. No formatter covers Markdown at all
There is no
.prettierrcand no.prettierignorein this repo, and neitherscript/fmtnorscript/fmt-checktouches Markdown. The convention across these repos is prettier with default configuration plus two exceptions — four-space indents andproseWrap: always(hard-wrap at 80 columns).This repo carries four Markdown files that are edited constantly —
README.md,TODO.md,TESTING.md,REPO_POLICIES.md— andTODO.mdis touched by every commit by policy. Every one of those edits has been hand-formatted to approximate the surrounding style, because there is no tool to do it. The implementer of #99 hit exactly this and said so in their PR.The result is drift that no gate can catch and no author can be blamed for.
Definition of done
script/fmt-checkverifies everythingscript/fmtapplies. For goimports, use its list mode (goimports -l) and fail with the offending filenames, mirroring the existinggofmt -lhandling so the output style stays consistent..prettierrcand.prettierignoreadded at the repo root..prettierrcsets four-space indentation andproseWrap: always, otherwise prettier defaults..prettierignoremust excludestatic/css/tailwind.min.css. It is 9KB of minified vendor output; reformatting it would produce an enormous meaningless diff and could plausibly break the embedded stylesheet the dashboard depends on. Exclude any other generated or vendored asset you find.script/fmtformats Markdown (and any other prettier-covered files not excluded);script/fmt-checkchecks them read-only viaprettier --check.script/bootstrapinstalls prettier at a pinned version, consistent with how golangci-lint and goimports are pinned. See the hard constraints below — this is the part most likely to go wrong.sh(#!/bin/sh,set -eu, no bashisms) — they run in minimal alpine images with no bash — and keep the$(cd "$(dirname "$0")/.." && pwd -P)root-location idiom.make fmt-checknow fails naming that file; introduce a badly wrapped Markdown paragraph and confirmmake fmt-checkfails on it. Revert both, confirmgit statusis clean, confirmmake checkis green. Report what you planted and what each produced. A code-reading argument is not sufficient — these are bugs about a gate not doing what it claims.make checkgreen;TODO.mdupdated in the same commit.The finishing commit's title must end with
(closes #N)referencing this issue.Hard constraints
REPO_POLICIES.mdand it has zero exceptions. If you add prettier, pin it exactly — a lockfile with integrity hashes, or a pinned container image bysha256. Nevercurl | sh, nevernpx prettierunpinned, never@latest.script/bootstrap, theDockerfile, and Docker build time (policy ceiling: 5 minutes). If the only clean way you can find to pin prettier drags a full Node install into this repo's bootstrap and image, stop and report back with what you found rather than committing it — that trade-off is worth a decision, not an assumption. A hash-pinned prettier container image used only byscript/fmt/script/fmt-checkmay be the lighter option; evaluate it..golangci.yml— sha256 must remain021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb.static/css/tailwind.min.css.Coordination
Touches
script/fmt,script/fmt-check, andscript/bootstrap. #117 also changesscript/bootstrap(making pinned installs actually take effect) and #115 changesscript/cibuild. All three are small; whichever lands later rebases. Keep them as separate PRs — do not merge them into one.Item 7 will conflict with any open PR that edits Markdown. At time of writing that includes PRs #97, #112, #113, and #118. Land this after those have merged, or expect to redo the reflow.