Summary
Two toolchain gaps that let avoidable defects through, both cheap to close.
1. No gofmt / go vet gate
.github/workflows/deploy.yml runs go test for all four modules but never checks formatting or runs go vet. On master (a06ac8ac):
$ gofmt -l . | wc -l
69
$ git ls-files '*.go' | wc -l
343
69 of 343 Go files (20%) are unformatted, including non-test source: cmd/server/analytics_recomputer.go, cmd/server/cdn_detection.go, cmd/server/analytics_warmup_1659.go, cmd/ingestor/decoder.go, cmd/ingestor/path_resolver.go, cmd/ingestor/source_status.go.
Most are comment-alignment drift, e.g.:
var cdnHeaders = []string{
- "CF-Connecting-IP", // Cloudflare
- "CF-Ray", // Cloudflare
+ "CF-Connecting-IP", // Cloudflare
+ "CF-Ray", // Cloudflare
}
Individually trivial, collectively noisy: every PR that touches one of these files either drags an unrelated reformat into the diff or leaves it inconsistent. A one-line CI step (test -z "$(gofmt -l .)") plus one sweep commit ends it permanently.
go vet is the same argument with higher stakes — it catches printf mismatches, lost struct tags, and unreachable code that go test will not.
2. ESLint is unrunnable on a clean checkout
eslint is not in package.json at all — neither dependencies nor devDependencies. The config is .eslintrc.json, the legacy format dropped in ESLint 9.
So on a fresh clone:
$ npx eslint public/*.js
npm warn exec The following package was not found and will be installed: eslint@10.7.0
Oops! Something went wrong! :(
ESLint: 10.7.0
ESLint couldn't find an eslint.config.(js|mjs|cjs) file.
Hard failure, no lint output. npx resolves to ESLint 10, which cannot read .eslintrc.json.
This matters because eslint public/*.js appears in the test plan of essentially every frontend PR in this repo — it is part of the expected contributor workflow. Today it only works for people who happen to have ESLint 8 installed globally from earlier. Anyone setting up fresh gets an error and either skips the step or, worse, migrates the config locally to make it run.
With ESLint 8.57.1 installed manually the config works fine and the codebase is clean: 0 errors, 90 warnings, all no-unused-vars on _-prefix-exempt names.
Suggested fix
- Pin
"eslint": "^8.57.1" in devDependencies and add "lint": "eslint public/*.js" to scripts, so npm run lint is reproducible. (Migrating to flat config is a larger, separate change — pinning is the one-line unblock.)
- Add
gofmt -l and go vet ./... steps to the go-test job in deploy.yml.
- One sweep commit for the 69 files, recorded in
.git-blame-ignore-revs (the repo already has one) so blame stays readable.
Happy to open PRs for any of these. Related: the CI/test-all.sh divergence filed separately.
Summary
Two toolchain gaps that let avoidable defects through, both cheap to close.
1. No
gofmt/go vetgate.github/workflows/deploy.ymlrunsgo testfor all four modules but never checks formatting or runsgo vet. Onmaster(a06ac8ac):69 of 343 Go files (20%) are unformatted, including non-test source:
cmd/server/analytics_recomputer.go,cmd/server/cdn_detection.go,cmd/server/analytics_warmup_1659.go,cmd/ingestor/decoder.go,cmd/ingestor/path_resolver.go,cmd/ingestor/source_status.go.Most are comment-alignment drift, e.g.:
var cdnHeaders = []string{ - "CF-Connecting-IP", // Cloudflare - "CF-Ray", // Cloudflare + "CF-Connecting-IP", // Cloudflare + "CF-Ray", // Cloudflare }Individually trivial, collectively noisy: every PR that touches one of these files either drags an unrelated reformat into the diff or leaves it inconsistent. A one-line CI step (
test -z "$(gofmt -l .)") plus one sweep commit ends it permanently.go vetis the same argument with higher stakes — it catches printf mismatches, lost struct tags, and unreachable code thatgo testwill not.2. ESLint is unrunnable on a clean checkout
eslintis not inpackage.jsonat all — neitherdependenciesnordevDependencies. The config is.eslintrc.json, the legacy format dropped in ESLint 9.So on a fresh clone:
Hard failure, no lint output.
npxresolves to ESLint 10, which cannot read.eslintrc.json.This matters because
eslint public/*.jsappears in the test plan of essentially every frontend PR in this repo — it is part of the expected contributor workflow. Today it only works for people who happen to have ESLint 8 installed globally from earlier. Anyone setting up fresh gets an error and either skips the step or, worse, migrates the config locally to make it run.With ESLint 8.57.1 installed manually the config works fine and the codebase is clean: 0 errors, 90 warnings, all
no-unused-varson_-prefix-exempt names.Suggested fix
"eslint": "^8.57.1"indevDependenciesand add"lint": "eslint public/*.js"toscripts, sonpm run lintis reproducible. (Migrating to flat config is a larger, separate change — pinning is the one-line unblock.)gofmt -landgo vet ./...steps to thego-testjob indeploy.yml..git-blame-ignore-revs(the repo already has one) so blame stays readable.Happy to open PRs for any of these. Related: the CI/
test-all.shdivergence filed separately.