REPO_POLICIES.md: "The Makefile is authoritative documentation for how the
repo is used… If someone checks out the repo and types make<tab>, they should
see every meaningful operation available."
The Makefile provides bootstrap setup test lint fmt fmt-check check docker hooks serve. It has no target for script/cibuild or script/precommit,
both of which exist, are documented in the README's Entrypoints section, and
are part of the standard entrypoint set.
Raised independently by two reviewers, most recently on PR #32 where the
reviewer had to invoke script/cibuild directly because no make target exists.
This matters more than it did before two recent changes:
Since #30, a bare docker build . fails by design (the CHECK_EPOCH guard),
and README.md now tells people to build the image only via script/cibuild
or script/docker. So the one build path a newcomer is directed to has no
make target, while make docker — the other path — does.
The global convention is to use make targets rather than invoking underlying
tools. That is impossible for the CI build as things stand.
Fix
Add thin shims, consistent with every existing target:
Check .PHONY is complete while there — it should list every target the Makefile defines.
Definition of done
make cibuild and make precommit exist and shim to the corresponding
scripts, matching the existing target style.
.PHONY lists every target defined in the Makefile.
make cibuild succeeds and its check layer genuinely executes — it must not
report CACHED on RUN ... make check (see #23/#30).
make precommit succeeds on a clean tree.
README Entrypoints section mentions the make targets where it currently
names only the scripts, so the two documents agree.
make check passes.
TODO.md updated.
Note
script/cibuild runs docker build, so make cibuild will be slow compared
with other targets. That is expected — it is the CI build — but worth a word in
the README so nobody types it expecting make check speed.
Out of scope
Changing what script/cibuild or script/precommit do.
The prettier-flag comment work, which is tracked in the sibling issue filed
alongside this one.
## Problem
`REPO_POLICIES.md`: "The Makefile is authoritative documentation for how the
repo is used… If someone checks out the repo and types `make<tab>`, they should
see every meaningful operation available."
The `Makefile` provides `bootstrap setup test lint fmt fmt-check check docker
hooks serve`. It has **no target for `script/cibuild` or `script/precommit`**,
both of which exist, are documented in the README's Entrypoints section, and
are part of the standard entrypoint set.
Raised independently by two reviewers, most recently on PR #32 where the
reviewer had to invoke `script/cibuild` directly because no make target exists.
This matters more than it did before two recent changes:
- Since #30, a bare `docker build .` fails by design (the `CHECK_EPOCH` guard),
and `README.md` now tells people to build the image only via `script/cibuild`
or `script/docker`. So the one build path a newcomer is directed to has no
make target, while `make docker` — the other path — does.
- The global convention is to use make targets rather than invoking underlying
tools. That is impossible for the CI build as things stand.
## Fix
Add thin shims, consistent with every existing target:
```make
cibuild:
@script/cibuild
precommit:
@script/precommit
```
and add both to `.PHONY`.
Check `.PHONY` is complete while there — it should list every target the
`Makefile` defines.
## Definition of done
1. `make cibuild` and `make precommit` exist and shim to the corresponding
scripts, matching the existing target style.
2. `.PHONY` lists every target defined in the `Makefile`.
3. `make cibuild` succeeds and its check layer genuinely executes — it must not
report `CACHED` on `RUN ... make check` (see #23/#30).
4. `make precommit` succeeds on a clean tree.
5. README Entrypoints section mentions the make targets where it currently
names only the scripts, so the two documents agree.
6. `make check` passes.
7. `TODO.md` updated.
## Note
`script/cibuild` runs `docker build`, so `make cibuild` will be slow compared
with other targets. That is expected — it is the CI build — but worth a word in
the README so nobody types it expecting `make check` speed.
## Out of scope
- Changing what `script/cibuild` or `script/precommit` do.
- The prettier-flag comment work, which is tracked in the sibling issue filed
alongside this one.
Two premises in the body are stale after 25b6c0a; DoD item 3 needs restating.
The Dockerfile no longer runs make check — it runs script/test then script/fmt-check, and lint runs in Dockerfile.lint. So "must not report CACHED on RUN ... make check" no longer names a real line. The requirement it stands for survives: make cibuild must be shown to execute its checks rather than return a warm-cache green, and script/cibuild now runs script/lint first, so there are two builds to account for, not one.
make lint now requires buildx. Do not paper over that here; it is #40 .
Everything else in the DoD stands. Scope stays exactly what the title says — Makefile shims plus the README wording that makes the two agree. Do not touch script/cibuild or script/precommit behaviour, and leave the README's script/lint entry alone; it is owned by #40 .
Two premises in the body are stale after `25b6c0a`; DoD item 3 needs restating.
- The `Dockerfile` no longer runs `make check` — it runs `script/test` then `script/fmt-check`, and lint runs in `Dockerfile.lint`. So "must not report `CACHED` on `RUN ... make check`" no longer names a real line. The requirement it stands for survives: `make cibuild` must be shown to execute its checks rather than return a warm-cache green, and `script/cibuild` now runs `script/lint` first, so there are two builds to account for, not one.
- `make lint` now requires buildx. Do not paper over that here; it is https://git.eeqj.de/sneak/lora.vegas/issues/40 .
Everything else in the DoD stands. Scope stays exactly what the title says — Makefile shims plus the README wording that makes the two agree. Do not touch `script/cibuild` or `script/precommit` behaviour, and leave the README's `script/lint` entry alone; it is owned by https://git.eeqj.de/sneak/lora.vegas/issues/40 .
make cibuild and make precommit added as thin shims; .PHONY lists both (it was already complete otherwise). README's Entrypoints section now carries the script-to-target mapping, the two names that differ (script/install-precommit is make hooks, script/precommit is make precommit), and the note that make cibuild costs minutes. The script/lint bullet was left alone per #40 .
Verification: make check green, make precommit green on a clean tree. make cibuild was shown to execute rather than replay cache — one invocation, 9.8s wall, ran both builds with distinct CHECK_EPOCH values, RUN script/bootstrapCACHED above and no check layer cached below: the Dockerfile.lint build echoed lint epoch: 17863691323256328833837425 and printed hugo's build table, then the main image build echoed check epoch: 17863691366690793943837425, ran script/test, and ran script/fmt-check for 3.7s printing All matched files use Prettier code style!.
Buildx was present on the machine used, so the make lint buildx dependency did not bite here.
Done in `341c8af` on `next`, carried by https://git.eeqj.de/sneak/lora.vegas/pulls/41 .
`make cibuild` and `make precommit` added as thin shims; `.PHONY` lists both (it was already complete otherwise). README's Entrypoints section now carries the script-to-target mapping, the two names that differ (`script/install-precommit` is `make hooks`, `script/precommit` is `make precommit`), and the note that `make cibuild` costs minutes. The `script/lint` bullet was left alone per https://git.eeqj.de/sneak/lora.vegas/issues/40 .
Verification: `make check` green, `make precommit` green on a clean tree. `make cibuild` was shown to execute rather than replay cache — one invocation, 9.8s wall, ran both builds with distinct `CHECK_EPOCH` values, `RUN script/bootstrap` `CACHED` above and no check layer cached below: the `Dockerfile.lint` build echoed `lint epoch: 17863691323256328833837425` and printed hugo's build table, then the main image build echoed `check epoch: 17863691366690793943837425`, ran `script/test`, and ran `script/fmt-check` for 3.7s printing `All matched files use Prettier code style!`.
Buildx was present on the machine used, so the `make lint` buildx dependency did not bite here.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
REPO_POLICIES.md: "The Makefile is authoritative documentation for how therepo is used… If someone checks out the repo and types
make<tab>, they shouldsee every meaningful operation available."
The
Makefileprovidesbootstrap setup test lint fmt fmt-check check docker hooks serve. It has no target forscript/cibuildorscript/precommit,both of which exist, are documented in the README's Entrypoints section, and
are part of the standard entrypoint set.
Raised independently by two reviewers, most recently on PR #32 where the
reviewer had to invoke
script/cibuilddirectly because no make target exists.This matters more than it did before two recent changes:
docker build .fails by design (theCHECK_EPOCHguard),and
README.mdnow tells people to build the image only viascript/cibuildor
script/docker. So the one build path a newcomer is directed to has nomake target, while
make docker— the other path — does.tools. That is impossible for the CI build as things stand.
Fix
Add thin shims, consistent with every existing target:
and add both to
.PHONY.Check
.PHONYis complete while there — it should list every target theMakefiledefines.Definition of done
make cibuildandmake precommitexist and shim to the correspondingscripts, matching the existing target style.
.PHONYlists every target defined in theMakefile.make cibuildsucceeds and its check layer genuinely executes — it must notreport
CACHEDonRUN ... make check(see #23/#30).make precommitsucceeds on a clean tree.names only the scripts, so the two documents agree.
make checkpasses.TODO.mdupdated.Note
script/cibuildrunsdocker build, somake cibuildwill be slow comparedwith other targets. That is expected — it is the CI build — but worth a word in
the README so nobody types it expecting
make checkspeed.Out of scope
script/cibuildorscript/precommitdo.alongside this one.
Two premises in the body are stale after
25b6c0a; DoD item 3 needs restating.Dockerfileno longer runsmake check— it runsscript/testthenscript/fmt-check, and lint runs inDockerfile.lint. So "must not reportCACHEDonRUN ... make check" no longer names a real line. The requirement it stands for survives:make cibuildmust be shown to execute its checks rather than return a warm-cache green, andscript/cibuildnow runsscript/lintfirst, so there are two builds to account for, not one.make lintnow requires buildx. Do not paper over that here; it is #40 .Everything else in the DoD stands. Scope stays exactly what the title says — Makefile shims plus the README wording that makes the two agree. Do not touch
script/cibuildorscript/precommitbehaviour, and leave the README'sscript/lintentry alone; it is owned by #40 .Done in
341c8afonnext, carried by #41 .make cibuildandmake precommitadded as thin shims;.PHONYlists both (it was already complete otherwise). README's Entrypoints section now carries the script-to-target mapping, the two names that differ (script/install-precommitismake hooks,script/precommitismake precommit), and the note thatmake cibuildcosts minutes. Thescript/lintbullet was left alone per #40 .Verification:
make checkgreen,make precommitgreen on a clean tree.make cibuildwas shown to execute rather than replay cache — one invocation, 9.8s wall, ran both builds with distinctCHECK_EPOCHvalues,RUN script/bootstrapCACHEDabove and no check layer cached below: theDockerfile.lintbuild echoedlint epoch: 17863691323256328833837425and printed hugo's build table, then the main image build echoedcheck epoch: 17863691366690793943837425, ranscript/test, and ranscript/fmt-checkfor 3.7s printingAll matched files use Prettier code style!.Buildx was present on the machine used, so the
make lintbuildx dependency did not bite here.