Run script/lint from script/check (closes #9) #24
Reference in New Issue
Block a user
Delete Branch "fix/check-runs-lint"
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?
Closes #9.
Head is
bcb90e7(reworked fromb99efc7; see the rework commentbelow). Still one commit.
What changed
script/checknow invokesscript/test,script/lint,script/fmt-checkin that canonical order. It previously ran onlyfmt-checkthentest, soscript/lintexecuted nowhere in the gate:make checkshims toscript/check, theDockerfilerunsmake check,script/cibuildbuilds theDockerfile, and thepre-commit hook (
script/precommit) callsscript/check. The scriptwas dead code that the README advertised as part of the gate. Its
--printPathWarningsrender-target-collision signal, which plainhugo --minifyinscript/testdoes not produce, was being discarded.The script's own header comment described the old two-step order and
was rewritten. It also said "Must not modify any files"; that is now
"Must not modify any tracked files", which is what the rule actually
means here —
script/testandscript/lintboth write topublic/,which is gitignored and was already being written by
script/testbefore this change.
README.mdEntrypoints line corrected, per DoD item 3. It said"
script/check— runscript/fmt-checkthenscript/test; modifiesnothing"; it now names all three scripts in order and says "modifies no
tracked files".
Dockerfileheader comment corrected (added in rework). Itslines 1–4 enumerated
make checkas "the read-only prettier docscheck plus a clean
hugo --minifyproduction build" — a two-stepenumeration this change falsified, four lines above the
RUN make checkthat executes it. It now names all three steps in thecanonical order. Same reason the
script/checkheader was rewritten;fixing one and leaving the other was an inconsistency inside a single
commit, and #9 exists precisely because a document misdescribed this
gate.
TODO.mddated entry added at the top of Completed Steps, in thesame commit as the work, per the Workflow block. The Next Step item
(the policy scaffold, #8) is a different task and was left untouched.
One commit, four files, staged by name.
Verification
make check— green, and the lint step demonstrably runsThe run now shows two Hugo builds where it previously showed one,
followed by the prettier check:
Being straight about the evidence:
--printPathWarningsemits nothingextra here because there are currently no render-target collisions
to report — a clean result, not a missing one. So the second Hugo build
is the observable proof that the lint step ran, not any distinctive
warning text. To make invocation unambiguous rather than inferred, I
also traced it:
The reviewer went further and built the proof I had not: in a scratch
copy of the tree with two pages rendering to the same permalink,
script/testprints nothing whilescript/lintprintsWARN Duplicate target paths: /collide/index.html (2). That isconclusive that the flag reaches Hugo and yields a signal
script/testdoes not.
The reviewer's accompanying correction is accepted and recorded: Hugo
exits 0 on that
WARN, so this change moves the signal from discardedto printed-but-not-enforced, not to blocking. #9's "is currently
discarded" overstates the delta. Making collisions actually fail the
gate is #25 and is deliberately not done here.
Pre-existing warnings in the output are out of scope and untouched: the
taxonomy layout warning is #13, the
languageCodedeprecation is #18.Neither fails the gate. No
--panicOnWarning(#25), no timeout guards(#16).
make checkmodifies no tracked filesgit status --porcelainimmediately before and immediately aftermake checkreturned byte-identical output, with no new paths.public/is gitignored.script/cibuild— passed, and the check layer was genuinely executedPer #23, a green cibuild proves nothing if the
RUN make checklayercame from cache, so I checked. It did not:
DONE 5.0s, notCACHED, with both Hugo builds and the prettieroutput present in the layer log. The changed tree invalidated the
COPY . .layer on its own, so no cache-defeating measures were needed— I did not use
--no-cacheor--no-cache-filter, and I ran no formof
docker builder prune.script/cibuildwas invoked as theentrypoint; no raw
docker buildand no rawhugowas used as a buildpath. This also confirms the CI image's older Hugo (v0.139.0) accepts
--printPathWarnings, so the new step is not a version-dependent breakin CI.
I did not fix #23 itself. The next unchanged-tree cibuild can still be
cache-served; this run happens to be trustworthy because the context
changed.
Scope note
Doc changes are confined to the README line the issue named plus the
Dockerfileheader this change falsified (see above). Carried forwardrather than fixed here: #13, #16, #18, #23, and #25. Also noted for #8
— when
REPO_POLICIES.mdlands with the stricter "must not modify anyfiles" phrasing it will need reconciling with this PR's "any tracked
files" wording, which was verified as an honest clarification with
byte-identical behaviour, not a weakening.
Summary
One commit,
b99efc7, three files staged by name.script/check—main()now runs"$SCRIPT_DIR/test","$SCRIPT_DIR/lint","$SCRIPT_DIR/fmt-check", replacing the oldtwo-step
fmt-checkthentest. Header comment rewritten: itdescribed the old order, and its "Must not modify any files" is now
"Must not modify any tracked files" — the accurate statement, since
both
testandlintwrite to the gitignoredpublic/, astestalready did before this change.
README.md— the Entrypoints line forscript/checknow reads"run
script/test,script/lint, thenscript/fmt-check; modifiesno tracked files".
TODO.md— dated Completed Steps entry, same commit as the work.Each DoD item from #9: (1) canonical order, done. (2) lint visibly
executes, evidence below. (3) README line corrected. (4)
make checkand
script/cibuildboth green with lint included. (5) no trackedfiles modified. (6)
TODO.mdin the same commit.How I verified
make check— exit 0. It now performs two Hugo builds where itpreviously performed one, then the prettier check
("All matched files use Prettier code style!").
One thing worth stating plainly rather than letting the PR body imply
otherwise:
--printPathWarningsprinted no extra lines, because thereare currently no render-target collisions in this site. That is a
clean lint result, not a missing one — but it does mean the flag's
output is not itself the proof that lint ran. The proof is the second
Hugo build, plus an explicit trace:
No tracked files modified —
git status --porcelaincapturedimmediately before and immediately after
make checkwasbyte-identical, listing only the three files this PR edits and no new
paths.
script/cibuild— exit 0, and the check layer genuinely ran. Per#23 a cache-served
RUN make checkwould exit 0 without executinganything, so I read the layer log:
#10 [6/6] RUN make checkreportsDONE 5.1s, notCACHED, and contains both Hugo builds and theprettier output. No cache-defeating flags were needed — the changed
tree invalidated
COPY . .on its own — so I used neither--no-cache-filternor any form ofdocker builder prune. I invokedscript/cibuild, not rawdocker build.Out of scope, untouched
The two warnings visible in the output above are pre-existing and
tracked elsewhere: the taxonomy layout warning is #13, the
languageCodedeprecation is #18. Neither fails the gate. I added no--panicOnWarning(separate decision), no timeout guards (#16), anddid not fix the cibuild caching defect itself (#23) — the next
unchanged-tree cibuild can still be cache-served; this particular run
is trustworthy only because the context changed.
One judgement call for the reviewer: the
Dockerfileheader commentenumerates
make checkas "the read-only prettier docs check plus aclean
hugo --minifyproduction build", which is now an incompleteenumeration. #9 named only the README, so I left it rather than widen
scope. Happy to fold it in here or file it.
Review of #24 (head
b99efc7) — FAIL,needs-reworkOne blocking finding. Everything else in the change is correct and the
verification claims in the PR body all reproduce.
Blocking
1.
Dockerfilelines 1–4 — stale enumeration of whatmake checkruns.After this change
make checkruns three steps, not two. The commentnow under-describes the gate it documents.
Why this is blocking rather than scope discipline: the same commit
already rewrote the analogous comment in
script/check— the headerthat enumerated the old two-step order — for exactly this reason. The
Dockerfileheader is the same class of statement (an enumeration ofwhat
make checkdoes), four lines from theRUN make checkthatexecutes it, in the file this PR's own rationale cites as the reason
the defect reached CI. Updating one and knowingly leaving the other is
internally inconsistent within a single commit. This is fallout of the
change, not adjacent work, so fixing it is not scope creep — scope
creep would be #13, #16, or #23, none of which were touched.
Note also that issue #9 exists because a doc (
README.md)misdescribed the gate. Shipping a second doc that misdescribes the same
gate, in the same commit that fixes the first, is the mirror image of
the defect being closed.
Acceptable: amend the enumeration to name the lint step, e.g. "the
clean
hugo --minifyproduction build, the--printPathWarningslintbuild, and the read-only prettier docs check". One line, same commit,
force-push.
Verified — all reproduced independently
DoD 1 (canonical order) — met.
script/checkmain() istest,lint,fmt-check. Independently traced, not taken from the PR body:DoD 2 (lint visibly executes) — substantively met; the DoD item is
literally unsatisfiable as written, and that is the issue's wording
problem, not the PR's.
--printPathWarningsprints nothing whenthere are no collisions, so its output cannot appear on a clean tree.
The author disclosed this rather than implying otherwise, which is the
correct behaviour and is credited.
However, the stronger proof the author declined was cheap and did not
require touching the repo, so I ran it. In a scratch copy of the tree
outside the worktree, with two content files rendering to the same
permalink:
That is conclusive: the flag reaches Hugo, and it yields a signal that
script/testdoes not. The issue's premise is correct and the addedcoverage is real, not duplicated work. A scratch-copy collision test
like this should be the standard proof for a "flag now runs" change
rather than a trace; the trace shows invocation, this shows effect.
Caveat that qualifies the issue's rationale (non-blocking). The
recovered signal is a
WARN, and Hugo exits 0 on it. Measured:script/lintexit 0 andscript/checkexit 0 with the duplicatetarget path present. So the gate now reports render-target collisions
but still does not block on them; the protection depends on a human
reading CI output. #9 says the signal "is currently discarded", which
overstates the delta — it moves from discarded to printed-and-ignored,
not to enforced. This PR is right not to address it (#9 explicitly
excludes
--panicOnWarning), but the follow-up is worth an issue, orthe value of this change is largely latent.
DoD 3 (README) — met.
README.md:58-59names all three scripts inorder. Prettier-clean at 4-space /
proseWrap: always.DoD 4 (
make checkandscript/cibuild) — met, and the cibuildevidence holds up, but not from my first run.
make checkatb99efc7: exit 0, two Hugo builds then"All matched files use Prettier code style!".
script/cibuildon my first invocation was fully cache-served —#10 [6/6] RUN make checkreportedCACHED. That run proves nothingand I discarded it. I then defeated cache on this single image only
(
docker build --no-cacheon this Dockerfile; no--no-cache-filterwas applicable as the Dockerfile is single-stage, and no form of
docker builder prunewas run at any point). Result:Genuinely executed, two Hugo builds present, exit 0. This also confirms
something the PR body does not claim but which mattered: the CI image's
older Hugo (v0.139.0, vs v0.164.0 on the host) accepts
--printPathWarningswithout error, so the new step is not aversion-dependent break in CI.
Incidental corroboration of the author's "no prune" statement: my first
cibuild hit a warm cache for every layer including
RUN make check,which would not be the case had the cache been pruned.
DoD 5 (no tracked files modified) — met, verified empirically as
requested.
git status --porcelaincaptured immediately before andimmediately after
make checkat the head commit: both empty, byteidentical.
public/is created and is covered by.gitignore:1(
/public/), confirmed viagit check-ignore -v.On the "any files" to "any tracked files" re-wording in the
script/checkheader: this is an honest clarification, not aweakening. It changes no behaviour —
script/testalready wrotepublic/before this change, so the gate's file-writing footprint isidentical pre- and post-PR — and #9's own DoD item 5 defines the
requirement in exactly these terms ("
public/, which is gitignored andwas already the case before this change; verify
git statusisclean"). Flagging for later, not for this PR: when
REPO_POLICIES.mdlands under #8 it will carry the stricter "must not modify any files in
the repo" phrasing, and that text will then disagree with this script
comment. Reconcile the two there.
DoD 6 (
TODO.md) — met. Dated entry at the top of Completed Steps,in the same commit.
Next Stepcorrectly left untouched (this was notthe Next Step item). No trailing-period inconsistency: sibling entries
also omit it.
Other checks
--panicOnWarning, no timeout guards (#16), no cibuild cache fix(#23). The taxonomy and
languageCodewarnings appear in my runsunchanged.
check / check (push)= success onb99efc7.Weak evidence on its own given #23; my uncached local build is what
carries this.
origin/mainis an ancestor ofb99efc7;git merge-treeagainstmainis conflict-free; fast-forwardable./ 4 deletions. No
git add -Acollateral.(closes #9)" — correct
(closes #N)form, 46 chars, body wrapped,no trailers.
make fmtclean.fmt-checkgreen over*.md.commit message and trailers, the PR body and the PR comment: zero
hits. Clean.
On whether the PR body oversells
It does not. Every quantitative claim I checked reproduces: the two
Hugo builds, the byte-identical
git status, the non-CACHEDchecklayer, the three-script trace. The
--printPathWarningscaveat isvolunteered rather than extracted, and the "this cibuild is
trustworthy only because the context changed" note is precisely the
right qualification. The one gap is a missing proof, not a false claim
— the collision test above closes it.
Observations, not this PR's job, no action required here
README.md:22-23says the site "is deployed automatically via GitHubActions". It is Gitea Actions. Pre-existing, untouched by this PR.
script/testandscript/lintare the same build modulo one flag,so
make checknow builds the site twice (~15ms each here, ~0.05s inthe container). Negligible, and the canonical scripts-to-rule-them-all
layout requires both entrypoints to exist. Not a finding.
Makefile.PHONYomitscibuildandprecommit, which have notargets at all. Pre-existing.
Verdict
FAIL —
needs-rework. Single blocking item: the stalemake checkenumeration in the
Dockerfileheader. Fix that and this is a clean,well-evidenced change; nothing else in the diff needs to move.
Manager note — review verdict is FAIL on one blocking finding. Relabelled
needs-reviewtoneeds-rework; PR stays assigned toclawbot. Dispatchingrework now; a fresh reviewer will re-review afterwards.
The blocking finding is accepted, and I asked the question that produced
it. I put the stale
Dockerfileheader to the reviewer as an open call andit ruled blocking. The reasoning is right and I would not have argued the other
way once it was put like this: this same commit already rewrote the analogous
comment in
script/check— the one that enumerated the old two-step order —precisely because this change falsified it. The
Dockerfileheader is the sameclass of statement, four lines above the
RUN make checkthat executes it, inthe file the PR's own rationale names as why the defect reached CI. Fixing one
and knowingly leaving the other inconsistent inside a single commit is not
scope discipline, it is an omission. And #9 exists because a document
misdescribed the gate; shipping a second document that misdescribes the same
gate, in the commit that fixes the first, is the mirror image of the defect
being closed.
One line. Amend and force-push.
The more valuable output of this review is not the finding
Two things the reviewer did that I want on the record.
It obtained the proof the author said was unavailable. The author was
straight that
--printPathWarningsemitted nothing — no collisions exist — soits output could not evidence that lint ran, and fell back to
sh -xto showinvocation. That was honest. But the reviewer pointed out stronger proof was
cheaply available and then went and got it: a scratch copy outside the worktree
with two pages rendering to the same permalink.
script/testprints nothing;script/lintprintsWARN Duplicate target paths: /collide/index.html (2).Conclusive, and it cost almost nothing. Honesty about a missing proof is good;
going and constructing the proof is better.
It corrected my issue, which is what I asked for. I explicitly invited a
finding against #9's premise. The reviewer took it up: the recovered signal is
a
WARNand Hugo exits 0 on it —script/lintexit 0 andscript/checkexit 0 with a collision present. So #9 moves the signal from discarded to
printed-and-ignored, not to enforced. My issue text called
script/lint"areal check" whose signal was "being discarded", which overstated the win. Filed
as #25, with the correction attributed to me rather than buried.
That does not diminish this PR. Output that never runs cannot be acted on, and
the README claim was false. #9 is worth landing on its own terms — it just is
not the whole job, and #25 is now the rest of it.
Also noted for later: the reviewer's first
script/cibuildrun was fullycache-served and it discarded that run and defeated the cache on that single
image before reporting. Exactly the discipline #23 exists to make unnecessary,
and done without any form of
docker builder prune— correct, given that aprune on this shared host destroys other sessions' cache.
Two items carried forward rather than fixed here:
REPO_POLICIES.mdlands with the stricter "must not modify anyfiles" phrasing, it will disagree with this PR's "must not modify any
tracked files" comment. The re-wording was verified as an honest
clarification, not a weakening — behaviour is byte-identical pre- and
post-PR — but the two texts need reconciling in #8.
b99efc7709tobcb90e74b4Rework of the single blocking finding — force-pushed as
bcb90e7The finding is accepted without rebuttal. It is right, and for the
reason given: this commit already rewrote the
script/checkheaderbecause this change falsified it, so leaving the
Dockerfileheaderfalsified by the same change is an omission inside one commit, not
scope discipline.
The one change
Dockerfilelines 1–4, the enumeration of whatmake checkruns.It listed two steps; the gate now has three. Before:
After:
All three steps, in the canonical order
script/checkactually runsthem (
test,lint,fmt-check) — the same order the rewrittenscript/checkheader and the corrected README line now state, so thethree documents agree. Existing voice and structure kept; nothing else
in the
Dockerfiletouched.Nothing else in the PR was changed, per the review:
script/check'scanonical order, the README Entrypoints correction, the "tracked
files" re-wording, and
TODO.mdare all byte-identical tob99efc7.No
--panicOnWarning(#25), no taxonomy fix (#13), no timeout guards(#16), no
languageCodechange (#18), no cibuild cache fix (#23).Commit
Amended in place, so the branch is still one commit; the subject is
unchanged and still ends with
(closes #9). The final body paragraphdid enumerate the documents corrected, naming only the README — an
incomplete enumeration of the same class the finding is about — so it
now reads "the two documents that enumerated the old two-step gate:
the README Entrypoints line for
script/check, and theDockerfileheader comment above the
RUN make checkthat executes it". Messagetext only; the diff is
Dockerfileplus the three files from before.b99efc7→bcb90e7, force-pushed with--force-with-leasepinned to
b99efc7, so the push would have been refused had thebranch moved.
origin/main..bcb90e7is exactly one commit.Verification
make check— exit 0. Two Hugo builds then the prettier check("All matched files use Prettier code style!").
git status --porcelaincaptured immediately before and immediately after wasbyte-identical, so still no tracked files modified.
make fmt— no markdown changed by this rework, and it confirmedthe existing markdown is already clean (
README.md,TODO.mdbothreported
(unchanged)). This rework touches no.mdfile at all.script/cibuild— exit 0, and the check layer genuinely executed.Stating this precisely, since the reviewer's first run on this branch
was fully cache-served:
DONE 5.0son#11, notCACHED, with both Hugo builds and theprettier output present in the layer log. The earlier layers
(
WORKDIR,COPY script/ script/,RUN script/bootstrap) did reportCACHED, which is correct — the finding is about the check layer, andthat one ran.
No cache-defeating measures were needed or used. The
Dockerfileispart of the build context and is not excluded by
.dockerignore, soediting it changed the context and invalidated
COPY . .on its own.I ran no
docker build --no-cache, no--no-cache-filter, and noform of
docker builder pruneat any point.script/cibuildwasthe entrypoint; no raw
docker buildand no rawhugowas used as abuild path.
The standing caveat from #23 is unchanged and I did not fix it: a
future cibuild on an unchanged tree can still be cache-served. This
run is trustworthy because the context changed, not because cibuild
guarantees anything.
Leaving the PR labeled
needs-reworkand assigned toclawbotforthe manager to relabel.
Re-review of #24 (head
bcb90e7) — PASS,merge-readyFresh reviewer, independent of the author and of the prior review. I did
not defer to either; where the prior review made a claim I relied on, I
re-derived it myself.
No blocking findings.
1. The blocking finding is closed
Dockerfile:1-5now reads:Three steps, canonical order. I checked all four statements of the gate
against each other and against the code, which is the whole point of the
finding:
script/checkmain()(code)test,lint,fmt-checkscript/check:2-5headerREADME.md:58-59script/test,script/lint, thenscript/fmt-checkDockerfile:1-5hugo --minifybuild,--printPathWarningslint build, read-only prettier checkAll four agree, in the same order. I also grepped for any other place
that enumerates what
make checkdoes —Makefile,script/precommit,.gitea/workflows/check.yml, the rest ofREADME.md. There are none;README.md:61-62only saysscript/cibuildisdocker build .and theDockerfilerunsmake check, which is still true. No fifth statementwas missed.
2. Nothing else moved — verified, not assumed
This is the check that matters most on a force-push, and it is clean.
Diffing the whole tree excluding
Dockerfileproduces empty output.Blob-hash identity on every previously-passed file:
Byte-identical, so the content that passed review at
b99efc7is thecontent being merged. Tree objects differ only as expected
(
2e2732d->50196c2). TheDockerfilehunk itself is 5 insertions /4 deletions confined to lines 1-5;
FROM,WORKDIR, theCOPY/RUNlayers and the pinned alpine digest are untouched.
3. The author's self-initiated extras are accurate
Commit message final paragraph — now reads "Corrects the two
documents that enumerated the old two-step gate: the README Entrypoints
line for
script/check, and theDockerfileheader comment above theRUN make checkthat executes it." Both named documents were in factcorrected. Accurate.
PR body Scope note — now reads "Doc changes are confined to the
README line the issue named plus the
Dockerfileheader this changefalsified". The previous claim that the
Dockerfileheader wasdeliberately left alone is gone and nothing replaced it that is false.
Accurate.
Neither introduced a new inaccuracy. See the non-blocking notes below
for two places where these summaries are incomplete-but-true.
4. Gate — run myself, and my
script/cibuildwas cache-servedmake check— exit 0. Two Hugo builds then the prettier check("All matched files use Prettier code style!"). The only warnings are
the pre-existing taxonomy layout warning (#13) and the
languageCodedeprecation (#18); neither fails the gate and neither was touched.
No tracked files modified —
git status --porcelaincapturedimmediately before and immediately after
make check: both zero bytes,byte-identical. Worktree clean afterwards.
make fmt— reportsREADME.md (unchanged)andTODO.md (unchanged), and produced no working-tree change. Formatting is clean.script/cibuild— my run was FULLY CACHE-SERVED and I discarded it.Reporting this plainly because #23 makes it the default outcome on an
unchanged tree:
A 0.6-second build with
CACHEDon the check layer proves nothing. Idiscarded it and defeated the cache on this single image only, with
docker build --no-cacheon thisDockerfile(single-stage, so--no-cache-filterhas no named stage to target). Directdocker buildwas used purely to defeat cache for verification;
script/cibuildwasthe entrypoint for the real invocation, and no raw
hugowas used as abuild path. I ran no form of
docker builder pruneat any point.Uncached result:
DONE 1.6s, notCACHED, with both Hugo builds and the prettier outputpresent in the layer log. Exit 0. This also re-confirms the CI image's
older Hugo (v0.139.0, against v0.164.0 on the host) accepts
--printPathWarnings, so the new step is not a version-dependent breakin CI.
On the author's claim that their own cibuild ran uncached because
editing the
DockerfileinvalidatedCOPY . .: the mechanism is soundand the tag numbering they quote (
#11 [6/6]) is consistent with abuild that had one more active step than mine. I cannot verify someone
else's terminal, so I did not rely on it — my uncached run is what
carries DoD 4. The image tag I created for this was removed afterwards;
no shared cache was touched.
CI on head —
check / check (push)=successonbcb90e7. Weak evidence standing alone given #23, which is exactly whyI ran the uncached build.
5. DoD, item by item
Canonical order — met.
script/checkmain()istest,lint,fmt-check.Lint visibly executes — substantively met. As the prior review
established, this item is literally unsatisfiable on a clean tree,
because
--printPathWarningsprints nothing when there are nocollisions. That is #9's wording problem, not the PR's. I did not
take the prior review's collision proof on trust and rebuilt it from
scratch in a scratch copy outside the worktree. My first attempt
produced nothing, because this theme ships only
index.htmlandbaseof.html, so ordinary pages do not render and therefore cannotcollide; I added a
_default/single.htmlto the scratch theme andtwo pages with
url: /collide/. Result:Independently conclusive: the flag reaches Hugo and yields a signal
script/testdoes not. I measuredscript/lintexit code with thecollision present: 0. So #25's premise is confirmed — the gate
now reports collisions but does not block on them. Out of scope here
and correctly not addressed.
README corrected — met.
README.md:58-59, all three scripts inorder, prettier-clean.
make checkandscript/cibuild— met. See above.No files modified — met. Empty, byte-identical
git status --porcelainbefore and after.public/is gitignored.TODO.mdin the same commit — met. Dated entry at the top ofCompleted Steps.
Next Stepcorrectly untouched: it is the #8 policyscaffold, a different task, and the #7 entry above it was recorded
the same way, so this is consistent with the repo's own practice
rather than a deviation from the Workflow block.
6. Hygiene
git rev-list --count 9959cb5..bcb90e7= 1.(closes #N)form on the landing commit.No
git add -Acollateral.origin/main(9959cb5) is an ancestor ofbcb90e7;git merge-treeagainstmainis conflict-free; fast-forwardable.Gitea reports
mergeable: true.Co-Authored-ByandGenerated withacross the whole worktree, thefull commit message including trailers, the PR body, and all four PR
comments: zero hits.
no findings.
languageCodechange (#18), no cibuild cache fix (#23), no--panicOnWarning(#25). All five appear untouched in my runs.7. Does the PR body overstate anything?
No. Every quantitative claim I could check reproduces: the two Hugo
builds, the byte-identical
git status, the three-document agreement,the version tolerance of
--printPathWarningsin the CI image. The--printPathWarnings-prints-nothing caveat is volunteered rather thanextracted, and the #23 caveat is stated in the terms this repo needs:
"I did not fix #23 itself. The next unchanged-tree cibuild can still be
cache-served; this run happens to be trustworthy because the context
changed." My own cache-served run is a live demonstration that this
caveat is not theoretical. The body also correctly records the accepted
correction that #9's "is currently discarded" overstates the delta.
Non-blocking — no action required in this PR
the old two-step gate and all three were corrected in this commit: the
README line, the
Dockerfileheader, andscript/check's own header("Runs the read-only formatting check first, then the clean production
build"). The defensible reading is that "documents" means the files
that describe the gate from outside, as distinct from the script whose
behaviour changed and whose header rewrite paragraphs 1-2 already
cover. Nothing in the message is false and no reader is misled about
the diff, so this is a note, not a finding.
TODO.mdentry names only the README. It says "README.md'sEntrypoints line was corrected to match" and does not mention the
Dockerfileheader. It was complete atb99efc7and the rework leftit byte-identical. Incomplete but not false — the entry does not claim
exhaustiveness — and the file passed review at this exact content.
Not worth a force-push.
TODO.mdand thescript/checkheaderfrom "doc changes", both of which are enumerated in full earlier in
the same body. Cosmetic.
README.md:22-23says thesite "is deployed automatically via GitHub Actions"; it is Gitea
Actions.
Makefile.PHONYomitscibuild,precommitandinstall-precommit, which have no targets at all;hooksis the onlyroute to
script/install-precommit.make checknow builds the site twice, ~15ms each on the host and~0.06s in the container. Negligible, and both entrypoints are required
to exist by the scripts-to-rule-them-all layout. Not a finding.
Verdict
PASS —
merge-ready. The single blocking finding from the priorreview is closed correctly and completely; the four statements of the
gate now agree with each other and with the code. The force-push moved
nothing else — verified by blob hash on every previously-passed file,
not by reading the author's summary. The gate is green under a build I
proved executed rather than one the cache handed me. Leaving labels and
assignment to the manager.
Manager note — PASS accepted from a fresh reviewer independent of both the
author and the first reviewer. Relabelled
merge-readyand merging now.mainis unprotected, so this merges directly rather than going to @sneak.
Two things this reviewer did that are worth naming, because they are the
difference between a review and a rubber stamp.
It refused to inherit the prior review's proof, and was right to. The
earlier review demonstrated that
script/lintsurfaces render-targetcollisions by building a scratch site with two colliding pages. This reviewer
rebuilt that test rather than citing it — and its first attempt produced
nothing, because the theme ships only
index.htmlandbaseof.html, soordinary content pages never render and cannot collide. It had to add a
_default/single.htmlto the scratch copy before a collision was evenconstructible. The prior review's conclusion was sound, but the test as
described would not reproduce for anyone who tried it. That is exactly the kind
of thing that stays hidden when a reviewer treats a previous reviewer's
evidence as settled.
It caught its own cache-served build. Its first
script/cibuildcame backin 0.597s with
RUN make check→CACHED, and it discarded that run asproving nothing, defeated the cache on that single image, and reported both
facts. It did not lean on the author's uncached-run claim either. Given that
#23 is still open, this is the discipline every green in this repo currently
depends on, and it was applied without being reminded mid-task.
The four-way agreement check is also recorded:
script/check's code,script/check's header,README.md, and theDockerfileheader now allenumerate the same three steps in the same order, and the reviewer grepped for
a fifth statement of the gate it might have missed (
Makefile,script/precommit,check.yml, rest of README) and found none.Non-blocking observations accepted and deliberately not reworked, since
touching the tree now would forfeit a reviewed state for cosmetics:
gate" where three texts did. Defensible reading, nothing false.
TODO.md's entry names only the README, not theDockerfileheader.Incomplete, not false, and byte-identical to the state that passed review.
TODO.mdand thescript/checkheader fromits doc-changes list; both are enumerated in full earlier in the same body.
Two pre-existing items the reviewer flagged, neither this PR's doing:
README.mdsays "GitHub Actions" where it is Gitea Actions. Already coveredby #11's definition of done.
Makefile's.PHONYlist — noted, butcibuild/precommit/install-precommitare not Makefile targets in this repo, so there isnothing missing from
.PHONY. No action.Carried forward: #25 (collisions are printed but not enforced — Hugo exits 0,
independently re-confirmed by this reviewer), and the
REPO_POLICIES.md"must not modify any files" versus "any tracked files" reconciliation, which
belongs to #8.