make check runs prettier --check . twice
#29
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?
Problem
script/checkrunsscript/test,script/lint,script/fmt-check. Butscript/lintalreadyruns
prettier --check .in addition to eslint, somake checkexecutes the same prettier passtwice. Visible in the output as two consecutive "All matched files use Prettier code style!"
blocks.
Pre-existing; spotted during review of #28 and deliberately left out of that PR's scope.
Definition of done
prettier --checkruns once permake check. Eitherscript/lintdrops it and stays eslint-only,or
script/checkstops callingscript/fmt-checkseparately — pick whichever keepsmake lintandmake fmt-checkmeaningful as standalone targets, sinceREPO_POLICIES.mdrequires both to exist and do what their names say.
make check,make lintandmake fmt-checkall still behave correctly when invoked on theirown.
make checkgreen.Not a 1.0.0 blocker
Wasted work, not wrong behaviour.
The ground has moved since this was filed: #30 has landed on
next, soscript/lintis now adocker buildofDockerfile.lint, and that container is the authoritative prettier verdict.script/fmt-checkstill runs prettier on the host. So the duplicate is no longer two identical host passes — it is one host pass and one container pass.That resolves the choice this issue left open. Drop the
script/fmt-checkcall fromscript/check(and checkscript/precommitfor the same duplication) rather than removing prettier fromscript/lint: the container has to keep checking formatting, because a successfulDockerfile.lintbuild is what CI treats as proof.make fmt-checkstays as a standalone target, which is whatREPO_POLICIES.mdrequires.Requirements:
make check, and a test that asserts it rather than a claim that it is so — this repo has a standing habit of guarantees nothing enforces.make lint,make fmt-checkandmake checkmust each still do what their name says when invoked alone.--frozen-lockfile, it reads.gitignoreby default, and.dockerignoremirrors it. Do not break that alignment; if your change makesmake fmt-checkthe only host-side formatting check, say what now catches a formatting problem before commit.script/cibuildstill green and shown to have actually executed, not served from cache.Implementation plan, following the ruling in the comment above (drop the host pass, keep the container one).
1.
script/check— remove the"$SCRIPT_DIR/fmt-check"line, leavingscript/testthenscript/lint. Update the header comment to say why:script/lintbuildsDockerfile.lint, which runsprettier --check .as a build step, so a separate host pass is a second, weaker verdict of the same thing.2.
script/precommit— same duplication, same fix. It currently runsscript/lintthenscript/fmt-check; drop the second. The hook keeps catching formatting problems, becausescript/lintis the container prettier run — so the pre-commit hook still does exactly what it did, minus one redundant pass.make fmt-checksurvives as a standalone target (REPO_POLICIES.mdrequires it), it just stops being wired intoscript/checkandscript/precommit.3. A test that enforces it,
test/packaging/lint-once.test.ts. Not a prose claim: it statically resolves the invocation graph rooted atscript/checkand counts prettier invocations. The resolver follows"$SCRIPT_DIR/<name>"andscript/<name>edges,make <target>edges through the Makefile shims,yarn run <name>edges throughpackage.jsonscripts, anddocker build -f Dockerfile.lintinto that file'sRUNsteps — comments stripped, since they name the very commands under assertion. Assertions:script/check, and exactly one fromscript/precommit;script/fmt-checkandscript/linteach still invoke prettier exactly once when invoked alone, so neither becomes a no-op that trivially satisfies the count;Dockerfile.lint, so a restructure that defeats the resolver fails the test instead of passing with a count of zero.I will mutate the assertion (re-add the
fmt-checkcall toscript/check) and confirm the test goes red before trusting it.4. Alignment check. Host and container prettier verdicts must stay unable to disagree, per review of #31: prettier pinned at 3.8.1, installed under
--frozen-lockfile, reading.gitignoreby default, with.dockerignoremirroring.gitignoreand deliberately keeping.gitignorein the context. Nothing in this change touches any of those four; I will re-verify and record it rather than assert it.5. README — the Entrypoints entries for
script/checkandscript/precommit, the pre-commit item in the development workflow, and the required-checks bullet all currently describefmt-checkas part of both. Corrected to match.6.
TODO.md— a Completed Steps entry; Next Step left alone (it is the unrelated README API reference).Verification:
make fmt, thenmake lint,make fmt-checkandmake checkeach invoked on their own, thenscript/cibuildwith timing and per-layer output shown so a cached green cannot be mistaken for an executed one. Prettier invocation counts recorded before and after.Done in commit
a73f0abonnext, which is carried by the open PR #31 (comment there has the full evidence).What was built.
script/checkno longer callsscript/fmt-check— it runsscript/testthenscript/lint, andscript/lintis the build ofDockerfile.lint, which runsprettier --check .as a build step.script/precommithad the same duplication and got the same fix.script/fmt-checkis unchanged and still wired tomake fmt-check; it is now a standalone entrypoint rather than a step inside two other scripts.test/packaging/lint-once.test.tsenforces the count instead of promising it. It walks the invocation graph from an entrypoint, following"$SCRIPT_DIR/<name>"andscript/<name>edges,make <target>through the Makefile shims,yarn run <name>through thepackage.jsonscripts, anddocker build -f <file>into that Dockerfile'sRUNsteps, with comments stripped. It asserts one prettier invocation permake checkand perscript/precommit, one each formake lintandmake fmt-checkalone so neither can satisfy the count by becoming a no-op, and which nodes the walk reached so a resolver that stops working fails loudly instead of counting zero.How it was verified.
make check, counted in the output of a real run: 2 before (one container, one host), 1 after. Both runs exit 0.script/fmt-checktoscript/check(expected 2 to be 1), deleting the prettier step fromDockerfile.lint(five failures), and guttingscript/fmt-check(still checks formatting under make fmt-check). It also caught a genuine bug in its own first draft, where asliceargument mistake made everymake-rooted walk return zero — the reachability assertions failed rather than the count silently passing.make fmt-checkexit 0 in 2s on the host;make lintexit 0 in 74s with eslint and prettier both executing in the container, not cached;make checkexit 0 in 18s, 258 tests passed.script/cibuildexit 0 in 19s, withCACHEDonly on the dependency layers above the epoch guards and every layer below them showing a real duration.On the one point this issue asked to be stated explicitly.
make fmt-checkis now the only host-side formatting check, but the thing that catches a formatting problem before a commit lands is unchanged: the pre-commit hook runsscript/precommit, which runsscript/lint, which is the container prettier run. It would have failed there before ever reachingscript/fmt-checkanyway. The hook lost a redundant pass, not a capability.Host and container verdicts still cannot disagree, re-verified and now asserted by the test: prettier pinned at an exact
3.8.1, installed under--frozen-lockfilein both places, and.gitignorestill in the build context because prettier reads it as a default ignore file. Nothing in this change touches any of the three.