Skip to content

docs: README check description now lists typecheck - #9

Merged
derek-miller merged 1 commit into
mainfrom
fl-11-readme-typecheck
Aug 12, 2026
Merged

docs: README check description now lists typecheck#9
derek-miller merged 1 commit into
mainfrom
fl-11-readme-typecheck

Conversation

@derek-miller

Copy link
Copy Markdown
Contributor

Fixes FL-11: the README still described make check as format-check and lint, but check gained typecheck as a prerequisite (shipped to make help in FL-10). Corrects the description and documents the make typecheck target. Docs-only, no behaviour change.

Ref: https://youtrack.dmiller.me/issue/FL-11

@derek-miller
derek-miller enabled auto-merge (squash) August 12, 2026 01:34

@svc-finitelabs svc-finitelabs Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: looks correct, ship it. Filing this as a comment rather than a formal approve: squash auto-merge was armed on this PR at 2026-08-12T01:34:17Z, reviewDecision is REVIEW_REQUIRED, and two matrix jobs are still running, so an approve from me is the last gate and would merge this itself once they go green. That trigger is yours to pull, not mine. Nothing here is a reservation about the change.

Verified every changed line against the Makefile at 3f03f1d rather than against the PR description:

  • check: format-check lint typecheck, so "format check, lint, and typecheck" is accurate.
  • format-check runs stylua --check, so "Check formatting without modifying" is accurate.
  • typecheck runs lua-language-server --check, and the new line matches the make help wording verbatim.
  • The block now matches the Code Quality section of make help in both contents and order, and the comment column stays at 26 across all five lines.
  • Check (the job that runs make check) is already green on the head; the Lua matrix jobs are still running but do not touch docs.

Two leftovers of the same drift, both out of scope here, worth a follow-up:

1. CLAUDE.md:62-63 is now the stale copy. It reads:

make help is stale on this point: it describes check as "format-check and lint" and omits typecheck.

be34f24 (#8) fixed make help, and CLAUDE.md has not been touched since f886c04, which lands before it. So the file now tells a reader that a correct help target is wrong. That is the same defect FL-11 exists to fix, one file over, and since it is the agent-facing doc it is the copy most likely to be acted on.

2. README.md:200 still reads # Install development dependencies (stylua, luacheck, amalg). install-deps also runs brew install lua-language-server stylua, and the block this PR just edited now documents make typecheck as needing lua-language-server. The setup line is where a reader would go looking for it and not find it.

Neither blocks this merge.

@svc-finitelabs

Copy link
Copy Markdown
Contributor

Follow-up filed for the first leftover in the review above: FL-14. Re-scoping it, since it is two repos rather than one: the same make help is stale sentence sits in lua-bthome-ble CLAUDE.md:80-81 as well as lua-crypto:62-63, and both files predate the FL-10 make help fix. lua-bitn, lua-noiseprotocol and lua-protobuf have no such sentence.

The second leftover, README:200's dev dependency list, is already tracked as FL-13, fleet-scoped.

All 11 check runs on 3f03f1d have since gone green, including the Lua matrix jobs that were still running when that review was posted.

@derek-miller
derek-miller disabled auto-merge August 12, 2026 02:36
@derek-miller

Copy link
Copy Markdown
Contributor Author

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.

@svc-finitelabs svc-finitelabs Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving now that auto-merge is off.

The earlier review on this same head (3f03f1d) was posted as a COMMENT rather than an approve only because squash auto-merge was armed at the time: with reviewDecision at REVIEW_REQUIRED, an approve would have been the merge rather than a verdict. Auto-merge was disabled at 02:37Z with the head unchanged, so the approve is now just the verdict. Nothing about the code changed the assessment.

Re-verified the three added/changed README lines against Makefile at 3f03f1d:

  • make check really is format-check lint typecheck (Makefile:188), so the old "format check and lint" description was understating it.
  • make format-check shells out to stylua --check (Makefile:137-140).
  • make typecheck shells out to lua-language-server --check (Makefile:177-180).

The block also now matches the Code Quality section of make help (Makefile:216-221) in contents and order: check, format, format-check, lint, typecheck.

All 11 contexts required by the main ruleset are green at this head (Check, Build Combined Module, Lua 5.1 through 5.4, luajit-2.0/2.1, and the three lua-openssl pins). The ruleset wants one approving review, which this supplies. Merge is yours to take.

Non-blocking, already tracked so no action needed here: README Setup still lists make install-deps as "(stylua, luacheck, amalg)" while the recipe also brews lua-language-server (Makefile:102), which now matters because this block documents make typecheck. That is FL-13, fleet-scoped. Separately, CLAUDE.md:62-63 still tells a reader that make help is stale on exactly this point, which stopped being true when #8 landed; that is FL-14.

@derek-miller
derek-miller merged commit bcf9c6f into main Aug 12, 2026
11 checks passed
@derek-miller
derek-miller deleted the fl-11-readme-typecheck branch August 12, 2026 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant