sneak/webhooker
1
0
Fork 0
Code Issues 11 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

No JS linting, so the styleguide REPO_POLICIES binds us to is unenforced #120

New Issue
Closed
opened 2026-08-11 14:45:30 +02:00 by clawbot · 2 comments
clawbot commented 2026-08-11 14:45:30 +02:00
Collaborator
Copy Link
Copy Source

REPO_POLICIES.md binds this repo to a JS styleguide ("Use const for everything... Never use var"), but nothing checks it. script/fmt-check is gofmt -s -l . only, and script/lint is golangci-lint — both Go-only. static/js/app.js is covered by no tooling at all.

This is not hypothetical: the review of #116 failed on five var declarations in static/js/app.js that a fully green make check and a cache-defeated script/cibuild both passed. The gate cannot see the file.

The JS surface here is small, which is the argument for fixing it now while it is cheap rather than after it grows.

Definition of done

  • A linter covers static/js/ and runs as part of script/lint, so make check and CI both fail on a styleguide violation.
  • It runs in Docker, consistent with #109's direction for lint tooling, and is pinned by digest per repo policy — not installed ad hoc on the host.
  • Acceptance check: reintroducing a var in static/js/app.js makes the gate go red. Without that demonstration the fix is unverified.

Implementation notes

  • Whatever tool is chosen should be configured to the styleguide REPO_POLICIES.md names, not to its own defaults.
  • Consider whether the existing lint container can host it rather than adding a second image.
  • Branch from next, PR based on next, single commit, title ending (closes #N).
`REPO_POLICIES.md` binds this repo to a JS styleguide ("Use `const` for everything... Never use `var`"), but nothing checks it. `script/fmt-check` is `gofmt -s -l .` only, and `script/lint` is golangci-lint — both Go-only. `static/js/app.js` is covered by no tooling at all. This is not hypothetical: the review of https://git.eeqj.de/sneak/webhooker/pulls/116 failed on five `var` declarations in `static/js/app.js` that a fully green `make check` and a cache-defeated `script/cibuild` both passed. The gate cannot see the file. The JS surface here is small, which is the argument for fixing it now while it is cheap rather than after it grows. ## Definition of done - A linter covers `static/js/` and runs as part of `script/lint`, so `make check` and CI both fail on a styleguide violation. - It runs in Docker, consistent with https://git.eeqj.de/sneak/webhooker/issues/109's direction for lint tooling, and is pinned by digest per repo policy — not installed ad hoc on the host. - Acceptance check: reintroducing a `var` in `static/js/app.js` makes the gate go red. Without that demonstration the fix is unverified. ## Implementation notes - Whatever tool is chosen should be configured to the styleguide `REPO_POLICIES.md` names, not to its own defaults. - Consider whether the existing lint container can host it rather than adding a second image. - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`.
clawbot commented 2026-10-03 02:32:57 +02:00
Author
Collaborator
Copy Link
Copy Source

Plan.

  • ESLint, at a pinned version from a committed package.json and lockfile, lints static/js/ (not the vendored Alpine.js). It runs only in Docker like the Go linter: a stage on a node image pinned by digest, reusing one the repo already pins if it has one. script/lint runs it, so make lint and make check fail on a violation, and a run counts as a pass only when the linter's own output shows it ran, as script/lint already requires of the Go linter.
  • The config states the rules of the JavaScript styleguide REPO_POLICIES.md links to, each one it can check (for example no-var, prefer-const), not ESLint's defaults alone.
  • Any violation already in static/js/app.js is fixed in the same change.
  • Acceptance: a var put back into static/js/app.js turns make check red; the PR says in one line that this was tried.

Model: opus-5-5

Plan. - ESLint, at a pinned version from a committed `package.json` and lockfile, lints `static/js/` (not the vendored Alpine.js). It runs only in Docker like the Go linter: a stage on a node image pinned by digest, reusing one the repo already pins if it has one. `script/lint` runs it, so `make lint` and `make check` fail on a violation, and a run counts as a pass only when the linter's own output shows it ran, as `script/lint` already requires of the Go linter. - The config states the rules of the JavaScript styleguide `REPO_POLICIES.md` links to, each one it can check (for example `no-var`, `prefer-const`), not ESLint's defaults alone. - Any violation already in `static/js/app.js` is fixed in the same change. - Acceptance: a `var` put back into `static/js/app.js` turns `make check` red; the PR says in one line that this was tried. Model: opus-5-5
clawbot commented 2026-10-03 02:56:26 +02:00
Author
Collaborator
Copy Link
Copy Source

Done in #486. ESLint, pinned by package.json and yarn.lock, now lints static/js/ in a js-lint stage of the Dockerfile on the digest-pinned node 24 LTS image. script/lint builds that stage, and the image build depends on it. eslint.config.mjs turns on the styleguide's two checkable rules, no-var and prefer-const. static/js/app.js already complied.

  • Deviation from the plan: script/lint does not look for ESLint's output, because ESLint prints nothing on a pass. It names the stage once for both --target and --no-cache-filter, so a renamed stage fails the build rather than replaying a cached pass.
  • Judgement call: one stage, so every make lint downloads ESLint again (hash-checked).
  • Not covered: the styleguide's prettier rule, a formatter's job (#215).
  • Tried: a var in static/js/app.js turns make check red.

Model: opus-5-5

Done in https://git.eeqj.de/sneak/webhooker/pulls/486. ESLint, pinned by `package.json` and `yarn.lock`, now lints `static/js/` in a `js-lint` stage of the `Dockerfile` on the digest-pinned node 24 LTS image. `script/lint` builds that stage, and the image build depends on it. `eslint.config.mjs` turns on the styleguide's two checkable rules, `no-var` and `prefer-const`. `static/js/app.js` already complied. - Deviation from the plan: `script/lint` does not look for ESLint's output, because ESLint prints nothing on a pass. It names the stage once for both `--target` and `--no-cache-filter`, so a renamed stage fails the build rather than replaying a cached pass. - Judgement call: one stage, so every `make lint` downloads ESLint again (hash-checked). - Not covered: the styleguide's prettier rule, a formatter's job (https://git.eeqj.de/sneak/webhooker/issues/215). - Tried: a `var` in `static/js/app.js` turns `make check` red. Model: opus-5-5
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
critical
Extremely important: data loss, a security hole, production down, or basic use impossible
 merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#120