Skip to content

refactor(cmd): 6 of 21 non-test files exceed the 500-line limit, up to 827 lines #1834

Description

@cristim

CLAUDE.md sets a 500-line file limit. Enumerated against origin/main (not a working tree), counting every non-test .go file under cmd/:

file lines
cmd/configure_gcp.go 827
cmd/helpers.go 788
cmd/multi_service_helpers.go 705
cmd/multi_service.go 698
cmd/configure_azure.go 602
cmd/multi_service_csv.go 509

6 of 21 non-test files in cmd/ are over the limit, four of them by 40% or more. Test files are excluded from the count above; several of those are larger still.

Why this is filed separately

Raised by CodeRabbit on #1825 as a Major maintainability finding against cmd/multi_service.go. That PR is a bug fix on a money path (--input-csv drives real commitment purchases, and the PR closes two blockers: a cap that selected alphabetically when savings data was absent, and a ranking key that bought the wrong commitments). Restructuring 698 lines of pre-existing purchase logic inside that PR would mean a large, hard-to-review refactor riding along with a correctness fix, which is the wrong risk to take on that path.

#1825 addresses its own contribution by extracting the ranking and cap logic it added into a separate file. The pre-existing baseline is this issue.

Fix direction

Split by bounded context rather than by line count, and one file per PR so each split is reviewable on its own:

  • cmd/multi_service.go — the CSV path (runToolFromCSV, filterAndAdjustRecommendations, and the cap/ranking helpers) is already a distinct context from the live-scan path (runToolMultiService, fetchExistingCoverage, the purchase pipeline).
  • cmd/helpers.go and cmd/multi_service_helpers.go are grab-bag names; the contents will suggest the seams.
  • cmd/configure_gcp.go / cmd/configure_azure.go likely split along prompt-flow versus API-call lines.

Verification bar

A split must be a pure move: no logic change in the same commit, so the diff can be read as "these functions moved" and nothing else. Run the full package test suite before and after and confirm identical results. Watch gocyclo -over 10 (the pre-commit threshold, stricter than golangci) since moving code can change what the linter attributes to a file. Do not combine a split with a behaviour change in one commit; that is precisely what makes large refactors unreviewable.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions