Skip to content

refactor(cmd): split multi_service_csv.go under the 500-line limit - #2295

Merged
cristim merged 1 commit into
mainfrom
refactor/1834-split-multi-service-csv
Oct 26, 2026
Merged

cristim merged 1 commit into
mainfrom
refactor/1834-split-multi-service-csv

Conversation

@cristim

@cristim cristim commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Pure move: the CSV cell formatting helpers (formatIntOrBlank to end of file) move from cmd/multi_service_csv.go (654 -> 471 lines) to the new cmd/multi_service_csv_formatters.go (190 lines). No logic, signature, comment or formatting change; only the import blocks differ.

Proof: stripped the package line, import blocks and blank lines from the original file and from the two new files, sorted all lines, and diffed. The two multisets are identical (578 lines each).

No non-test file in cmd/ exceeds 500 lines after this change.

go test -race -count=1 ./..., go vet ./... and golangci-lint pass.

Closes #1834

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • CSV recommendation reports now display zero-valued metrics as blank, unknown coverage as “n/a,” and known coverage to one decimal place. Monthly recurring cost remains blank when unavailable.
    • Recommendation details such as instance families, database deployment, and engine or platform are extracted for supported recommendation types.
  • Refactor
    • CSV formatting and recommendation detail handling have been reorganized without changing the available report capabilities.

Pure move of buildTotalRow-adjacent formatting helpers (formatIntOrBlank
through the end of the file) into multi_service_csv_formatters.go. Only
the import block differs. Part of #1834.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 0429536 and 84937c4.

Files selected for processing: 2
  • cmd/multi_service_csv.go
  • cmd/multi_service_csv_formatters.go
Files with no reviewable changes: 1
  • cmd/multi_service_csv.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

Walkthrough

CSV formatting and recommendation-detail helper functions were moved from cmd/multi_service_csv.go into cmd/multi_service_csv_formatters.go. The new file contains helpers for extracting recommendation fields and formatting numeric, currency, coverage, and instance values.

Changes: CSV formatter extraction

Layer / File(s) Summary
Extract and format CSV helper functions
cmd/multi_service_csv.go, cmd/multi_service_csv_formatters.go
The original file no longer contains the AWS recommendations import or the CSV helper functions after buildTotalRow. The new file defines helpers for extracting recommendation details and formatting CSV values, including normalized units, coverage, recurring cost, currency, and instance counts.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: iamjsd

Merge Risk: Minimal · up to 84937

No actionable issue is identified with the CSV formatter extraction; it is ready for normal merge checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "refactor(cmd): split multi_service_csv.go under the 500-line limit" matches the main change. The PR moves CSV cell-formatting helpers out of cmd/multi_service_csv.go into the new cmd/multi_…
Linked Issues check Passed Issue #1834 requires a pure split of cmd/multi_service_csv.go by bounded context. The whole-PR diff removes the CSV formatting helpers and the AWS recommendations import from the original file, then…
Out of Scope Changes check Passed The whole-PR diff changes only cmd/multi_service_csv.go and adds cmd/multi_service_csv_formatters.go. These changes implement the multi_service_csv.go split requested by issue #1834. The remaini…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 1 files.
Full details: Title check

Explanation

The title "refactor(cmd): split multi_service_csv.go under the 500-line limit" matches the main change. The PR moves CSV cell-formatting helpers out of cmd/multi_service_csv.go into the new cmd/multi_service_csv_formatters.go, which brings the original file under the 500-line limit. The title names the refactor, the scope (cmd), and the file. It does not cover the other oversized files in issue #1834, but the PR does not claim to fix those either.

Full details: Linked Issues check

Explanation

Issue #1834 requires a pure split of cmd/multi_service_csv.go by bounded context. The whole-PR diff removes the CSV formatting helpers and the AWS recommendations import from the original file, then adds the same helpers to cmd/multi_service_csv_formatters.go with the required imports. The diff shows no helper logic, signature, comment, or formatting change. The PR reports that go test -race -count=1 ./..., go vet ./..., and golangci-lint pass. The reported result also leaves no non-test file in cmd/ above 500 lines.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff changes only cmd/multi_service_csv.go and adds cmd/multi_service_csv_formatters.go. These changes implement the multi_service_csv.go split requested by issue #1834. The remaining oversized files are not changed, as expected for the issue's one-file-per-PR scope.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 26, 2026

Copy link
Copy Markdown
Member Author

Review verdicts at 84937c4: two independent cold reviews (non-author), both APPROVE, no findings.

  • Pure move: the removed tail of cmd/multi_service_csv.go (base lines 473-654) is byte-identical and in the same order as cmd/multi_service_csv_formatters.go:9-190. The only other change is the providers/aws/recommendations import, which moved with the code.
  • There are no init(), build tags or generated-code markers.
  • gofmt, go vet, golangci-lint and go test -race -count=1 ./... are clean.
  • No non-test .go file exceeds 500 lines at head.

[claude-cc-libs]

@cristim
cristim merged commit 3d8e4cd into main Oct 26, 2026
12 checks passed
@cristim
cristim deleted the refactor/1834-split-multi-service-csv branch October 26, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant