chore(cli): kubeconfig-flag helper + pinned lint/deadcode gate + dead-export cleanup (#127)#235
Open
LukasWodka wants to merge 1 commit into
Open
chore(cli): kubeconfig-flag helper + pinned lint/deadcode gate + dead-export cleanup (#127)#235LukasWodka wants to merge 1 commit into
LukasWodka wants to merge 1 commit into
Conversation
…+ unexport dead symbols Refs #127 (three tech-debt quick-wins; the optional data-delete CLI-layer tests remain out of scope). ITEM 1 — addKubeconfigFlags helper (internal/cli/flags.go): Replace the inline --kubeconfig/--context registrations duplicated across client.go, cluster.go, data.go, data_delete.go, data_list.go, delete.go and doctor.go with a single shared helper, plus a separate addNamespaceFlag so commands without a namespace (e.g. `client create`) don't grow a spurious -n. Pure refactor: flag names/shorthands/defaults/usage strings are byte-identical (shared constants where the phrasing already matched; per-command literals kept where it differed). Diff limited to the registration lines. ITEM 2 — pinned lint tools + deadcode gate (Makefile + build.yml): Pin errcheck@v1.20.0, ineffassign@v0.2.0, misspell@v0.3.4 (was @latest) in both the Makefile and the CI workflow, and refresh the now-stale "unpinned" comment. Add a deadcode (golang.org/x/tools/cmd/deadcode@v0.48.0) gate over ./cmd/tracebloc. The gate is ADVISORY (continue-on-error / `|| true`) for now: of the 7 funcs it reports, 4 are unsafe or out-of-scope to remove — two Stringer methods reached only via fmt reflection (Status.String, JobOutcome.String) and two di#349 test-only parity harnesses (ReadLabelValues, inferColumnType) — and the 3 ITEM-3 symbols stay test-reachable-only, so a blocking gate would red-fail CI. Flip to blocking once that backlog clears (tracked in #6 / #127). ITEM 3 — unexport test-only exports: config.Clear -> clearAll (clear is a Go builtin), push.AllCategoryIDs -> allCategoryIDs, submit.IsSubmitError -> isSubmitError. Each had zero production callers and is referenced only by same-package tests, so unexport (not delete) per the ticket; tests updated to match. Gates from a clean worktree: go build, go vet, gofmt -l (empty), go test ./... (all green), scripts/coverage-floor.sh (cli 73.9% >= 68%, submit 76.1% >= 72%), pinned errcheck/ineffassign/misspell (clean), deadcode (advisory). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
The remaining tech-debt items of cli#127. (The
resolveClusterTargethelper and thedataset→datarename sweep already shipped; this covers the rest.)Refs #127rather than Closes — the issue's optional "CLI-layer tests for the destructivedata deletepath" box stays open.ITEM 1 — shared
addKubeconfigFlagshelper. Newinternal/cli/flags.gocentralizes the--kubeconfig/--contextregistration (plus a separateaddNamespaceFlagfor-n, kept separate soclient createdoesn't gain a spurious-n) that was duplicated inline across 7 command files (client,cluster,data,data_delete,data_list,delete,doctor). Pure refactor — flag names/shorthands/defaults/usage byte-identical, proven by diffing--helpoutput for all 7 commands built from both branches (identical).ITEM 2 — pinned lint +
deadcodegate.errcheck/ineffassign/misspellpinned (were@latest) in both the Makefile andbuild.yml; the stale "Tool versions are unpinned" comment removed; added adeadcodegate (golang.org/x/tools/cmd/deadcode@v0.48.0) as amake deadcodetarget + CI step. The gate is advisory (non-blocking) for now: deadcode flags 7 funcs, but 4 are out-of-scope/unsafe to remove — di#349 CSV/ingestor parity harnesses used only by tests (ReadLabelValues,inferColumnType), and twoString()Stringer methods reached viafmtreflection that static analysis can't see (Status.String,JobOutcome.String) — and the other 3 are the ITEM-3 test-only symbols below. A code comment documents how to flip it blocking once that backlog is cleared.ITEM 3 — dead test-only exports unexported.
config.Clear→clearAll(clearis a Go builtin),push.AllCategoryIDs→allCategoryIDs,submit.IsSubmitError→isSubmitError— all had zero production callers (referenced only by same-package tests), so unexported (not deleted) with test call-sites updated.Tests / gates
go build/vetOK;gofmt -lclean;go test ./...green;coverage-floor.sh(cli 73.9% ≥ 68%, submit 76.1% ≥ 72%); the pinned linters run clean;deadcodeadvisory (7 findings, documented).Merge-conflict note
ITEM-1 touches the flag-registration block in
data.go/data_delete.go/doctor.go/delete.go, which several in-flight PRs (#229/#230/#231 + the delete branches) also touch — but the diff is limited to theFlags()lines and those PRs mostly editRunE/logic, so the risk is low-moderate and mechanical to resolve.Refs #127.
🤖 Generated with Claude Code
Note
Low Risk
Mostly refactor and tooling; CLI flag behavior is intended to be byte-identical, and unexported symbols had no production callers.
Overview
#127 tech debt: centralizes duplicated
--kubeconfig/--context(and optional-n/--namespace) registration in newinternal/cli/flags.go, wired from seven commands with the same defaults and help text as before.CI/Makefile: pins
errcheck,ineffassign, andmisspell(replacing@latest) with version vars kept in sync between the Makefile andbuild.yml; adds an advisorydeadcodescan from./cmd/tracebloc(continue-on-errorin CI,|| trueinmake deadcode) and includes it inmake ci.Dead exports: unexports test-only symbols—
config.Clear→clearAll,push.AllCategoryIDs→allCategoryIDs,submit.IsSubmitError→isSubmitError—with same-package tests updated; no production behavior change.Reviewed by Cursor Bugbot for commit ad80846. Bugbot is set up for automated code reviews on this repo. Configure here.