Uh oh!
There was an error while loading. Please reload this page.
ci: re-enable staticcheck (pinned standalone) + fix the 5 non-ST1005 findings - #302
Merged
Conversation
LukasWodka
commented
Jul 14, 2026
ContributorAuthor
👋 Heads-up — Code review queue is at 39 / 30 Above the WIP limit. The team convention is to review existing PRs before opening new work. Open PRs currently in Code review (oldest first):
Pull from review before opening new work. (This is a nudge from the kanban WIP check, not a block.) |
5 tasks
LukasWodka
commented
Jul 14, 2026
ContributorAuthor
@BugBot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 88626ef. Configure here.
6 tasks
saadqbal
previously approved these changes
Jul 14, 2026
saadqbal
left a comment
Collaborator
There was a problem hiding this comment.
👍 The 5 fixes are all mechanical initialism/underscore renames (behavior-preserving) — verified no stale names linger and all builds/Lint/Test are green with staticcheck now running all,-ST1005. Sensible to exclude ST1005 given the user-facing sentence-style error strings.
10 tasks
saadqbal
commented
Jul 14, 2026
Collaborator
This was referenced Jul 14, 2026
LukasWodkaforce-pushed
the
ci/279-staticcheck-pinned
branch
from
July 14, 2026 15:08
88626ef to
7fe0519Compare
This was referenced Jul 14, 2026
Merged
Merged
Merged
Merged
LukasWodkaforce-pushed
the
ci/279-staticcheck-pinned
branch
from
July 14, 2026 16:34
9470567 to
8397831Comparesaadqbal
approved these changes
Jul 14, 2026
This was referenced Jul 14, 2026
…findings Pinned staticcheck@2025.1.1 joins the standalone lint set in both the Makefile lint target and build.yml's lint job (-checks all,-ST1005), mirroring the errcheck/ineffassign pin pattern. The golangci-lint OOM story that originally disabled it is stale for the standalone binary: a full run is ~12s wall locally. Findings fixed (the full non-ST1005 set): - ST1008: runSet (resources_set_test) returns (string, error), error last - ST1003: CheckMaskIdColumn -> CheckMaskIDColumn (+ call sites/comments) - ST1003: errors_as -> errorsAs, io_eof_or_similar -> ioEOFOrSimilar - ST1020: CoreFloorText doc comment starts with the function name ST1005 stays excluded: it flags ~58 customer-visible error strings that need a wording review, not a mechanical sweep — separate follow-up. Rebased onto develop: union-merged the lint job with develop's govulncheck job (#278), coverage-floor step (#301) and deadcode advisory gate; kept the audit comment #286 added above CheckMaskID*Column. The 'going back to the action' comment in build.yml is KEPT (not deleted) — #6 is still open on develop, so golangci-lint-action stays disabled and that rationale still holds. Detached the four data_ingest_*.go header comments (#303 split) plus exitcodes.go's (#284, which develop merged after this branch was cut) from the package clause so the newly-enabled staticcheck gate is ST1000-clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…monthly) (#304) * ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly) goimports -local github.com/tracebloc/cli now blocks in both loops: build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as the deadcode pin) and the Makefile's fmt-check target, with make fmt extended to auto-fix. .golangci.yml already declared this grouping via local-prefixes but nothing enforced it — 4 files had drifted (data.go, data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here with import-grouping-only diffs. .github/dependabot.yml extends the org's backend-only Dependabot pilot: gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped), github-actions monthly. Unlike backend's security-only config, the CLI takes real version updates — customers install this binary, so staying current is security posture (see #276). Org-wide rollout decision flagged to Asad on the PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: canonicalize import grouping in data_test.go for the goimports gate Regroups imports into the layout goimports -local github.com/tracebloc/cli emits (stdlib / third-party / tracebloc-local). gofmt and goimports only sort within existing blank-line groups, so the manual groups passed fmt-check as-is but were not the canonical single-block output; Bugbot and our precheck both flagged the divergence. Verified idempotent under the pinned goimports v0.48.0 and green on make fmt-check + go build ./... Rebased onto ci/279 (post-#303 data.go split): data.go's import block is already canonical from the #303 split, so its canonicalization here is a no-op and dropped — this commit now regroups data_test.go only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…313) * ci: goimports -local gate + Dependabot config (gomod weekly, actions monthly) goimports -local github.com/tracebloc/cli now blocks in both loops: build.yml's lint job (pinned goimports@v0.48.0, same x/tools version as the deadcode pin) and the Makefile's fmt-check target, with make fmt extended to auto-fix. .golangci.yml already declared this grouping via local-prefixes but nothing enforced it — 4 files had drifted (data.go, data_test.go, ingestion_run_test.go, resources_set_test.go), fixed here with import-grouping-only diffs. .github/dependabot.yml extends the org's backend-only Dependabot pilot: gomod weekly (k8s.io/* + sigs.k8s.io/* grouped, golang.org/x/* grouped), github-actions monthly. Unlike backend's security-only config, the CLI takes real version updates — customers install this binary, so staying current is security posture (see #276). Org-wide rollout decision flagged to Asad on the PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: canonicalize import grouping in data_test.go for the goimports gate Regroups imports into the layout goimports -local github.com/tracebloc/cli emits (stdlib / third-party / tracebloc-local). gofmt and goimports only sort within existing blank-line groups, so the manual groups passed fmt-check as-is but were not the canonical single-block output; Bugbot and our precheck both flagged the divergence. Verified idempotent under the pinned goimports v0.48.0 and green on make fmt-check + go build ./... Rebased onto ci/279 (post-#303 data.go split): data.go's import block is already canonical from the #303 split, so its canonicalization here is a no-op and dropped — this commit now regroups data_test.go only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: flip deadcode gate to blocking; delete the 3 dead #127 leftovers The deadcode CI step loses continue-on-error (and the Makefile target its '|| true'): both now run scripts/deadcode-check.sh, which fails on any function unreachable from ./cmd/tracebloc that isn't declared in scripts/deadcode-allowlist.txt. The tool itself always exits 0, so the old advisory step never could have blocked — the gate keys on output. Deleted (verified still dead with the pinned deadcode@v0.48.0): - config.clearAll + its three dedicated tests (TestClearAll, TestClearAll_HomeError, TestClear) — logout uses Save, nothing else ever called it - push.allCategoryIDs — moved verbatim into category_registry_test.go (the registry-pinning tests legitimately iterate every id; the shipped binary shouldn't carry the helper) - submit.isSubmitError — moved into client_test.go as the test-local assertion helper it always was (orphaned 'errors' import dropped) Allowlisted with reasons (the 4 legit findings): Status.String + JobOutcome.String (fmt-reflection Stringers) and ReadLabelValues + inferColumnType (di#349 test-only parity harnesses). Stale allowlist entries warn without failing; line numbers are stripped so edits that shift code don't red the gate. Coverage floors still clear after the test deletions (cli 82.9% >= 68, submit 80.3% >= 72). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
LukasWodkaforce-pushed
the
ci/279-staticcheck-pinned
branch
from
July 14, 2026 17:17
8397831 to
5a49f44Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WS-A.2 of epic tracebloc/backend#1106: re-enable staticcheck as a pinned standalone tool (
staticcheck@2025.1.1,-checks all,-ST1005) in both the Makefilelinttarget and build.yml's lint job, mirroring the existing errcheck/ineffassign/misspell pin pattern — and fix the full set of 5 non-ST1005 findings so the gate lands blocking and green.The original disable story (golangci-lint's SSA analysis OOM/time-budgeting the runner on this module's k8s.io-heavy dep tree) is stale for the standalone binary: a full cold
go run …staticcheck@2025.1.1 -checks all,-ST1005 ./...is a few seconds wall locally (~3s warm viamake lint). The stale build.yml comment about "going back to the action when #6 has a strategy" is deleted.Findings fixed (verified by running the pinned tool — exactly the 5 the ticket predicted)
internal/cli/resources_set_test.go:45runSetnow returns(string, error)— error last (all ~20 call sites swapped)internal/push/preflight.go:518CheckMaskIdColumn→CheckMaskIDColumn(exported; call sites + comments in push package updated, incl.TestCheckMaskIDColumn)internal/schema/validate.go:267errors_as→errorsAs(+ coverage-test references)internal/cli/ingest_test.go:250io_eof_or_similar→ioEOFOrSimilarinternal/resources/set.go:168CoreFloorTextdoc comment now starts with the function nameAll renames are identifier-only — no behavior change.
Deliberately NOT in this PR (follow-up)
ST1005 (error-string style) stays excluded: it flags ~58 findings, most of them customer-visible error strings whose wording deserves a deliberate review, not a mechanical sweep. Noted in both the Makefile and build.yml comments; needs its own ticket/PR.
Test plan (ran locally on this branch)
go run honnef.co/go/tools/cmd/staticcheck@2025.1.1 -checks all,-ST1005 ./...— 0 findings (was 5)go build ./...go test -race ./internal/cli/... ./internal/push/... ./internal/schema/... ./internal/resources/...— all greenmake lintend-to-end (errcheck + ineffassign + misspell + staticcheck) — green, ~3s warmgofmt -s -l .clean; build.yml YAML-parsesCoordination note
Expected trivial rebase vs sibling PRs touching build.yml's lint job / the Makefile (WS-A.3 #280 and WS-A.4 #281 stack on this branch) — merge order: this PR → #280's PR → #281's PR, but each is a surgical edit so any order rebases trivially.
Fixes#279
🤖 Generated with Claude Code
Note
Low Risk
Changes are mostly CI/Makefile gates and mechanical renames with no runtime behavior change; risk is low aside from new PR failures if future code adds unreachable symbols outside the allowlist.
Overview
Re-enables staticcheck as a pinned standalone gate (
staticcheck@2025.1.1,-checks all,-ST1005) in Makefilelintand build.yml, aligned with the existing errcheck/ineffassign/misspell pin pattern. ST1005 stays off until customer-facing error strings get a deliberate pass.Fixes the five findings that blocked a green gate:
runSetreturns(string, error)(ST1008),CheckMaskIDColumn/errorsAs/ioEOFOrSimilarnaming (ST1003), andCoreFloorTextdoc comment (ST1020). Renames are identifier-only.The same diff also tightens hygiene: goimports -local in CI and
make fmt/fmt-check, Dependabot for gomod (grouped k8s / golang.org/x) and github-actions, blocking deadcode viascripts/deadcode-check.shplus an allowlist for four known false positives, and removal or relocation of unreachable helpers (clearAll,isSubmitError,allCategoryIDs) so the binary stays free of test-only entrypoints. Import grouping tweaks in splitdata_ingest_*files are formatting-only.Reviewed by Cursor Bugbot for commit 5a49f44. Bugbot is set up for automated code reviews on this repo. Configure here.