Nothing looks for dead code, duplication, tangled functions or a value written from too many places #166

New Issue

2026-10-06T05:14:30Z

cgalo5758 commented

2026-10-06 05:14:30 +00:00

What a contributor runs into

make lint runs the project's own UI checks (go run . lint) and scripts/notebook-citations.sh, and make test runs the lint before the tests. Nothing in the repository looks for code nobody calls, copied blocks, functions too tangled to read, or a value that too many places write. Those are found by reading, if at all: #30 is two generated queries nothing calls, and #165 is a usage counter that eleven call sites raise and lower until it drifts.

Why it costs

Each of these is cheap to stop when it first appears and expensive to find later. The counter in #165 took several rounds of review to be recognised as one problem rather than many separate bugs.

Where

The Makefile's lint target and a configuration file at the repository root. Go has no single tool for all of this, but two cover it:

golangci-lint, as the one command: unused and unparam (code and parameters nothing uses), dupl (copied blocks), gocognit (cognitive complexity), and forbidigo, which can confine a call to named files. A rule that only one file may call AtomicIncrementUsage would have stopped the writers in #165 from multiplying; no analyzer finds the race itself.
deadcode (golang.org/x/tools/cmd/deadcode), which follows calls from main and reports exported functions nothing reaches, which unused cannot see. It runs as deadcode -test ./....

Import cycles and unused imports are already compile errors in Go, so they need nothing. Where these checks run automatically is #14.

Done when

make lint runs golangci-lint from a committed configuration, and deadcode.
Every finding on the existing code is fixed or recorded with a reason it stays, and new code fails on any finding.
At least one single-writer rule exists, starting with the usage counter once #165 is fixed.
The contributing guide names the checks.

## What a contributor runs into `make lint` runs the project's own UI checks (`go run . lint`) and `scripts/notebook-citations.sh`, and `make test` runs the lint before the tests. Nothing in the repository looks for code nobody calls, copied blocks, functions too tangled to read, or a value that too many places write. Those are found by reading, if at all: #30 is two generated queries nothing calls, and #165 is a usage counter that eleven call sites raise and lower until it drifts. ## Why it costs Each of these is cheap to stop when it first appears and expensive to find later. The counter in #165 took several rounds of review to be recognised as one problem rather than many separate bugs. ## Where The Makefile's `lint` target and a configuration file at the repository root. Go has no single tool for all of this, but two cover it: - golangci-lint, as the one command: `unused` and `unparam` (code and parameters nothing uses), `dupl` (copied blocks), `gocognit` (cognitive complexity), and `forbidigo`, which can confine a call to named files. A rule that only one file may call `AtomicIncrementUsage` would have stopped the writers in #165 from multiplying; no analyzer finds the race itself. - `deadcode` (`golang.org/x/tools/cmd/deadcode`), which follows calls from `main` and reports exported functions nothing reaches, which `unused` cannot see. It runs as `deadcode -test ./...`. Import cycles and unused imports are already compile errors in Go, so they need nothing. Where these checks run automatically is #14. ## Done when - `make lint` runs golangci-lint from a committed configuration, and `deadcode`. - Every finding on the existing code is fixed or recorded with a reason it stays, and new code fails on any finding. - At least one single-writer rule exists, starting with the usage counter once #165 is fixed. - The contributing guide names the checks.

cgalo5758 added this to the Public launch milestone 2026-10-06 05:14:30 +00:00

cgalo5758 added the

kind

debt

area/testing area/meta

priority

high

labels 2026-10-06 05:14:30 +00:00

cgalo5758 referenced this issue

2026-10-06 18:11:39 +00:00

An application outside the console cannot connect to it: there is no published protocol #171

cgalo5758 commented

2026-10-06 18:12:18 +00:00

Order: this comes first, then #165, then the integration contract in four parts: #171, #172, #173, #174.

cgalo5758 referenced this issue

2026-10-06 18:12:18 +00:00

FedWiki's site usage is a running total that many code paths adjust; derive it instead #165

cgalo5758 referenced this issue

2026-10-06 22:49:03 +00:00

FedWiki's site usage is a running total that many code paths adjust; derive it instead #165

cgalo5758 commented

2026-10-06 23:00:20 +00:00

The fixes run in six steps. Each exception in .golangci.yml names the step that removes it, or #165 for the three FedWiki functions #165 rewrites and the usage-counter test. This issue closes when the last step lands and no exception names it.

1. The gate: .golangci.yml, deadcode and a check for unused sqlc queries in make lint, and the exception list. Every finding other than complexity and test duplication is fixed here.
2. The test kit: shared helpers for test databases, HTML and sign-in; one template-set constructor per surface (#190); a transaction runner and a pool-lock helper (part of #186).
3. Member plans: one package decides which plans a member may move to, and the Products page, Checkout and the plan switch all call it (#188).
4. Operator reads: each operator page loads, derives and renders in separate functions, and the grants listing becomes one query (#189). The loops over an organization's pools in internal/server/operator_enrollment.go (the Pools panel in loadOrgEnrollmentData, IssueGrant, and subscriptionHeldLadders behind the Issue grant warning) move to the shared position read here; #188 closed without them.
5. Domain acts and billing: plan-wide ladder changes move out of HTTP handlers into the entitlements package (#187), and the rule-change, reconcile and Stripe sync functions split into named steps.
6. Libraries and the integration core: forms, the project lint, the sign-in callback, seeding and the integration core.

The fixes run in six steps. Each exception in `.golangci.yml` names the step that removes it, or #165 for the three FedWiki functions #165 rewrites and the usage-counter test. This issue closes when the last step lands and no exception names it. - [x] 1. The gate: `.golangci.yml`, deadcode and a check for unused sqlc queries in `make lint`, and the exception list. Every finding other than complexity and test duplication is fixed here. - [x] 2. The test kit: shared helpers for test databases, HTML and sign-in; one template-set constructor per surface (#190); a transaction runner and a pool-lock helper (part of #186). - [x] 3. Member plans: one package decides which plans a member may move to, and the Products page, Checkout and the plan switch all call it (#188). - [x] 4. Operator reads: each operator page loads, derives and renders in separate functions, and the grants listing becomes one query (#189). The loops over an organization's pools in `internal/server/operator_enrollment.go` (the Pools panel in `loadOrgEnrollmentData`, `IssueGrant`, and `subscriptionHeldLadders` behind the Issue grant warning) move to the shared position read here; #188 closed without them. - [ ] 5. Domain acts and billing: plan-wide ladder changes move out of HTTP handlers into the entitlements package (#187), and the rule-change, reconcile and Stripe sync functions split into named steps. - [ ] 6. Libraries and the integration core: forms, the project lint, the sign-in callback, seeding and the integration core.

cgalo5758 referenced this issue

2026-10-07 07:58:47 +00:00

There is no way to review which organizations a rule change recomputed #113

cgalo5758 referenced this issue

2026-10-07 07:58:47 +00:00

Two generated domain queries have no caller outside their own tests #30

cgalo5758 referenced this issue

2026-10-08 07:49:29 +00:00

FedWiki's site usage is a running total that many code paths adjust; derive it instead #165

cgalo5758 referenced this issue

2026-10-09 03:44:20 +00:00

Locks are written out at each call site, with four key conventions and no written order #186