Skip to content

chore: add a root Makefile so one command verifies the whole repo - #193

Merged
qiuethan merged 3 commits into
stagingfrom
chore/root-makefile
Aug 9, 2026
Merged

chore: add a root Makefile so one command verifies the whole repo#193
qiuethan merged 3 commits into
stagingfrom
chore/root-makefile

Conversation

@qiuethan

@qiuethan qiuethan commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

What this changes

A root Makefile. make check runs lint, format checks, and the fast test suites across all seven Python directories and the bot.

make help       list targets
make install    uv sync --extra dev  +  npm ci
make test       fast suites everywhere (no Docker, no network, no credentials)
make test-full  adds the Postgres adapter suites (needs Docker + migrations)
make lint       ruff check + ruff format --check + bot ESLint/Prettier
make format     apply ruff --fix / ruff format / eslint --fix / prettier
make check      lint + test — the pre-push command

Why

Verifying a change meant eight invocations across eight directories, each subtly different:

  • three services need --ignore=tests/test_postgres_adapter.py for the fast path, three don't
  • packages/auth is neither a service nor the bot
  • the bot is npm, not uv

That's a lot of ceremony for a repo whose contributors turn over every year, and the cost of forgetting a step is a red PR. Found during an onboarding-readiness sweep.

Not a second source of truth

Stated in the file header and in both docs: .github/workflows/ci.yml remains authoritative. If the Makefile and CI ever disagree, CI is right and the Makefile is the bug. It exists so nobody has to memorize eight command variants, not to define what "passing" means.

Merge-order independence

The bot's lint hooks are invoked as npm run --if-present lint / format:check. Those scripts only exist once #192 merges, so --if-present makes this work either way — no dependency on which PR lands first, and it starts enforcing automatically once #192 is in.

Zone

root — plus doc updates in README.md, AGENTS.md, docs/DEVELOPMENT.md, which is where the pre-push instructions live.

How to verify

make help
make lint     # clean across all 7 Python dirs
make format   # no-op, leaves no diff
make test

Results on my machine:

  • make lint — clean everywhere.
  • make format — no-op, git status clean afterward.
  • make test — 857 Python tests pass (224 / 199 / 58 / 61 / 136 / 139 / 40), then fails the bot on Node 18 with node:test missing its mock export.

That last failure is the Makefile working correctly, not a defect in it: the repo requires Node 20 and my local Node is 18. It's also a fair demonstration of the point — one command surfaced a setup problem that eight scattered ones let hide. The follow-up PR enforces the Node floor so the failure explains itself.

🤖 Generated with Claude Code

Verifying a change meant eight invocations across eight directories, each
slightly different: three services need --ignore=tests/test_postgres_adapter.py
for the fast path, three don't, packages/auth is neither a service nor the bot,
and the bot is npm. That is a lot to hold for a repo with rotating maintainers,
and forgetting a step means a red PR.

`make check` now runs lint, format checks, and the fast suites everywhere.
`make help` lists the rest: install, test, test-full, format, clean-pyc.

Explicitly a convenience wrapper, not a second source of truth — the header
says so, and so do the docs. ci.yml stays authoritative; if the two disagree,
the Makefile is the bug.

The bot's lint hooks go through `npm run --if-present`, so this works both
before and after the ESLint/Prettier PR lands rather than depending on merge
order.

Verified: `make lint` and `make format` clean across all seven Python
directories (format is a no-op, no diff). `make test` passes 857 Python tests
and then correctly fails the bot on Node 18 — which is the Makefile doing its
job, and the reason the Node-floor PR exists.
Two review findings, both about the Makefile promising more than it delivers.

1. `make check` is not CI-equivalent, but the docs said "run this before you
   push" as though it were. It runs the fast suites and skips the
   Postgres-adapter tests CI runs: 34 in team-tracking, 26 in
   documentation-system, 5 in verification, plus 3 behind RUN_PG_TESTS. 68
   tests. Someone editing a storage adapter could go green locally and still
   land a red PR -- the exact outcome the Makefile is supposed to prevent.

   Reworded in the Makefile header, `make help`, README, AGENTS.md and
   DEVELOPMENT.md: `make check` for most work, `make test-full` after touching
   storage, an adapter, or a migration.

2. Added a check-node preflight. The bot's failure on an old Node is
   unrecognisable -- `node:test` reports no `mock` export, fastify wants
   `diagnostics.tracingChannel` -- and neither message mentions Node, so the
   flagship command failed for reasons a newcomer cannot act on. It now reads
   the floor from discord-bot/package.json engines (one place to bump) and
   fails with what is wrong and how to fix it. Runs before the bot step only,
   so a Node problem never blocks Python work.

   Verified on Node 18.12 against the declared >=20 floor:

     The bot needs Node >=20 (from discord-bot/package.json engines).
     You are on v18.12.0, so its tests and lint will fail
     in ways that do not mention Node at all.

     Fix: nvm install 20 && nvm use 20

   `make lint` now runs all seven Python packages clean, then stops there.

Also added a "First run: make install" hint to help -- without it `uv run ruff`
fails with "Failed to spawn: ruff", which is the same species of unhelpful
error.
@qiuethan

qiuethan commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator Author

Review follow-up: it was over-promising

Two fixes. I also verified two things I suspected were bugs and they're fine: the set -e loop does abort on the first failing package, and clean-pyc with zero matches exits 0 rather than failing the target.

1. make check is not CI-equivalent, and the docs implied it was

It runs the fast suites and skips the Postgres-adapter tests CI runs:

Service Skipped by make check
team-tracking 34
documentation-system 26
verification 5
+ RUN_PG_TESTS-gated 3
Total 68

I'd written "run this before you push" into the README, AGENTS.md, and DEVELOPMENT.md — which reads as an assurance it can't give. Someone editing a storage adapter passes locally and still goes red on CI, the exact failure this PR claims to prevent.

Reworded everywhere: make check for most work, make test-full after touching storage, an adapter, or a migration.

2. Node preflight

The bot's failure on an old Node is unrecognisable — node:test reports no mock export, fastify wants diagnostics.tracingChannel — and neither mentions Node. So the flagship command failed for a reason a newcomer can't act on.

check-node reads the floor from discord-bot/package.json engines (one place to bump, and it stays correct regardless of what that floor becomes). Verified on Node 18.12:

  The bot needs Node >=20 (from discord-bot/package.json engines).
  You are on v18.12.0, so its tests and lint will fail
  in ways that do not mention Node at all.

  Fix: nvm install 20 && nvm use 20

It runs immediately before the bot step, not up front, so a Node problem never blocks Python work — make lint now completes all seven Python packages clean and then stops there.

Also added a First run: make install hint, since without it uv run ruff fails with Failed to spawn: ruff — same species of unhelpful error.

@qiuethan
qiuethan merged commit d2c199c into staging Aug 9, 2026
11 checks passed
@qiuethan
qiuethan deleted the chore/root-makefile branch August 9, 2026 20:10
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