chore(discord-bot): gate JS on ESLint + Prettier, matching the ruff pair - #192
Conversation
The bot was the one zone with no style enforcement. Every Python service is
gated on `ruff check` and `ruff format --check`; `node-test` ran only
`node --test`, leaving ~80 JS files unlinted. With rotating maintainers that
asymmetry drifts.
Adds ESLint 9 (flat config) for correctness and Prettier for formatting, with
eslint-config-prettier last so the two can't disagree. `printWidth` is 100 to
match the `line-length = 100` the Python services already set for ruff. Both
tools are scoped to `*.js` on purpose — the Markdown in this repo is
hand-wrapped and Prettier must not reflow it.
`node-test` now runs `npm run lint` and `npm run format:check` after the suite.
No new CI job, so the ten required checks are unchanged.
The bulk of this diff is the one-time reformat. It is formatting-only: running
Prettier over staging and diffing against this branch yields exactly the five
unused-variable removals ESLint flagged, and nothing else --
- src/linkService.js linkByEmail destructured an unused discordHandle
- test/teamService.test.js TeamNotFound imported, never used
- test/meetingClient.test.js / test/recorder.test.js (x2)
assigned call results never read; the calls stay
`ignoreRestSiblings` is on, so the existing `const { flags, ...editable }` omit
idiom stays legal.
Verified: npm test 33/35 -- identical to the pre-change baseline on this
machine. The two failures are Node 18 only (`node:test` has no `mock` export;
fastify needs `diagnostics.tracingChannel`) and pass on CI's Node 20.
…gnore
Two follow-ups from reviewing the config rather than trusting it.
1. The browser block didn't do what it claimed. Flat config MERGES
languageOptions.globals across every block whose `files` match, so layering
a browser block over a `**/*.js` node block gave src/web/public/ both sets.
Playground code could reference process, Buffer or __dirname and lint clean,
then break in the page.
Probed with deliberate errors, before -> after:
process.env in a browser file not flagged -> error
document.title in a node file error -> error
typo (documnt) in browser file error -> error
Fixed by splitting rules from globals and giving the node block an
`ignores: ['src/web/public/**']` so the two environments can't overlap.
Real codebase still lints and format-checks clean.
2. Added .git-blame-ignore-revs. The reformat touched 60 files, and without
this `git blame` there answers "who ran Prettier" instead of "who wrote
this" -- the wrong answer in a repo whose whole point is surviving turnover.
The file ships with the mechanism and instructions but no SHA yet, on
purpose: this repo squash-merges, so the branch SHA won't exist on staging
after merge. It carries a TODO to append the squashed SHA in a follow-up.
Review follow-up: fixed a real bug in the configReviewing this instead of trusting it turned up a defect worth catching before merge. The browser block didn't do what it claimedESLint flat config merges Probed with deliberate errors:
Fixed by splitting the rules block from the globals blocks and giving the node one Small blast radius — 2 files, and genuine typos were always caught — but the config asserted an isolation it didn't provide.
|
Follow-up to #192, which could not carry its own SHA: this repo squash-merges, so the commit did not exist until it landed on staging. It is c970bc9. Measured effect on the worst-hit files -- lines git blame credited to the formatter, before -> after: src/commands/team.js 55 of 188 -> 1 src/adapters/discord.js 40 of 591 -> 0 src/commands/doc.js 30 of 128 -> 1 src/web/public/app.js 23 of 304 -> 0 src/meeting/meetingSurface.js 22 of 259 -> 0 Nearly a third of team.js blamed on a formatting pass. Now it points at whoever wrote the line. GitHub applies this automatically in its blame view. Locally it is opt-in once per clone: git config blame.ignoreRevsFile .git-blame-ignore-revs
1 participant
What this changes
The discord-bot now has lint and format enforcement, gated in CI, mirroring the
ruff check+ruff format --checkpair every Python service already carries.eslint.config.js) for correctness only..prettierrc.json) for formatting,printWidth: 100to match ruff'sline-length = 100.eslint-config-prettierlast in the config array, so ESLint never argues with Prettier about style.lint,lint:fix,format,format:check.node-testrunsnpm run lintandnpm run format:checkafternpm test. No new job — the ten required status checks are unchanged.Why
The bot was the only CODEOWNERS zone with no style enforcement at all: ~80 JS files, no eslint, no prettier, no lint script, and a CI job that ran
node --testand nothing else. Every Python service is fully gated. With rotating student maintainers, that asymmetry is exactly where style drifts.Found during an onboarding-readiness sweep of the repo.
Zone
discord-bot— plus a 7-line step addition to.github/workflows/ci.ymland the doc updates below. It spans zones only because enforcement is meaningless without the CI hook; splitting them would merge a linter nobody runs.About the diff size
~3,300 lines, and all but 5 of them are the one-time reformat.
This is verifiable rather than a promise. Running Prettier over
stagingand diffing the result against this branch yields exactly five changes, all of them the unused variables ESLint flagged:src/linkService.jslinkByEmaildestructureddiscordHandleand never used it (its siblingconfirmAndLinkdoes)test/teamService.test.jsTeamNotFoundimported, never usedtest/meetingClient.test.jsopenStreamresult never read — call retainedtest/recorder.test.js(x2)startedRecorderresult never read —awaitretainedNothing else differs. Per AGENTS.md a formatting-only diff is fine on its own; this is that, plus five named dead-variable removals.
ignoreRestSiblingsis enabled so the existing omit idiom (const { flags, ...editable } = dpayload) remains legal — that pattern is deliberate, not dead code.How to verify
To confirm the reformat is semantically inert, reproduce the check above:
npm testis 33/35 on my machine — identical to the pre-change baseline. Both failures are Node 18 only (node:testexports nomock; fastify needsdiagnostics.tracingChannel) and pass on CI's Node 20. A follow-up PR enforces the Node floor so this stops being a trap.Docs updated
AGENTS.md— bot pre-push command; the lint-gating note now covers JSdocs/DEVELOPMENT.md— lint step in "Make your first contribution"docs/DEPLOYMENT-HISTORY.md—node-testjob descriptiondiscord-bot/docs/CONTRIBUTING.md— pre-push checklist🤖 Generated with Claude Code