docs: README check description now lists typecheck - #19
Conversation
There was a problem hiding this comment.
Reviewed at head 41c471c. Docs-only, and it matches the code.
Verified
Makefile:172ischeck: format-check lint typecheck, so the new wording atREADME.md:143is accurate.- Both halves of FL-11's lua-bitn scope are covered: the stale description is corrected, and
make typecheckis now documented. FL-11 called out that the four stale READMEs never mentioned the target at all, so adding that line was part of the ask, not extra. - The branch predates #18 (
f6db30a), so the branch's ownMakefilestill carries the pre-fixmake helpstring at line 201. That is not a revert: the three-dot diff is README-only, and mergingorigin/mainwith this branch locally leavesMakefile:201on main's corrected text. Flagging it only so the stale line in the branch file view is not mistaken for a regression.
Non-blocking, out of scope
README.md:112 describes make install-deps as installing "(stylua, luacheck, amalg)", but it also installs lua-language-server (Makefile:98). That is the binary make typecheck requires, and the target exits 1 without it (Makefile:161-169). Now that the README documents the target, a reader following the setup line has no signal that its dependency is already covered. Filed as FL-13 rather than growing this PR: same three repos are stale, lua-protobuf already lists it, lua-bthome-ble has no such line. Small enough to fold in here if you would rather not carry a second sweep.
Sibling sweep
lua-crypto#9, lua-bthome-ble#11, and lua-noiseprotocol#21 are open on the same branch name and title. lua-protobuf correctly has none, since FL-11 recorded its README as already accurate.
Why this is a comment and not an approve
Squash auto-merge is armed on this PR and reviewDecision is REVIEW_REQUIRED, so an approving review from me would be the last gate and would merge this to main itself. My guardrail is that I do not merge without your approval, so the verdict is here and the trigger stays yours. On content I have no objection: this is correct and ready.
|
Auto-merge is now disarmed on this PR, so an approving review is no longer the merge trigger. Content is verified correct per your review, so please go ahead and post the approve. I will merge manually once it lands and CI is green. |
There was a problem hiding this comment.
Approving now that auto-merge is off.
The earlier review on this same head (41c471c) was a COMMENT rather than an approve only because squash auto-merge was armed at the time, which would have made the approve the merge instead of a verdict. Auto-merge is off and the head has not moved, so this is just the verdict. The assessment is unchanged.
Re-verified against the branch Makefile: check: format-check lint typecheck (:172), with a real typecheck target shelling out to lua-language-server (:161). The new README wording matches the Code Quality block of make help on main (:201). The branch copy of that help line is still the pre-fix string only because the branch predates #18, which the earlier review already worked through, and a local merge leaves main's corrected text in place.
All required contexts are green at this head. Merge is yours to take.
The make install-deps residual raised earlier stays where it is, tracked as FL-13.
Fixes FL-11: the README still described
make checkas format-check and lint, butcheckgainedtypecheckas a prerequisite (shipped tomake helpin FL-10). Corrects the description and documents themake typechecktarget. Docs-only, no behaviour change.Ref: https://youtrack.dmiller.me/issue/FL-11