refactor(output): move lint rendering into internal/output/lint - #81
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds ChangesLint output migration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/output/lint/lint.go (1)
24-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrite the formatted finding directly to
builder.Line 32 creates an intermediate string and violates staticcheck QF1012. Use
fmt.Fprintfto write directly tobuilder.Proposed fix
- builder.WriteString(fmt.Sprintf("%s %s %s:%d %s", finding.Severity, finding.Rule, finding.File, finding.Line, finding.Message)) + fmt.Fprintf(&builder, "%s %s %s:%d %s", finding.Severity, finding.Rule, finding.File, finding.Line, finding.Message)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/output/lint/lint.go` around lines 24 - 35, Update the findings formatting loop to replace the intermediate fmt.Sprintf call with fmt.Fprintf writing directly to builder, while preserving the existing finding fields, prefix, separators, and trailing newline behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/output/lint/lint.go`:
- Around line 24-35: Update the findings formatting loop to replace the
intermediate fmt.Sprintf call with fmt.Fprintf writing directly to builder,
while preserving the existing finding fields, prefix, separators, and trailing
newline behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8bd953b-9e46-4f2a-bedf-1c4455e79d1f
📒 Files selected for processing (6)
internal/cli/lint.gointernal/output/doc.gointernal/output/lint/lint.gointernal/output/lint/lint_test.gointernal/report/json.gointernal/report/text.go
There was a problem hiding this comment.
Pull request overview
This PR refactors lint output rendering by moving the text and JSON renderers from internal/report into a new layered output adapter at internal/output/lint, keeping CLI orchestration separate from typed result rendering while preserving existing output formats and behavior.
Changes:
- Added
internal/outputandinternal/output/lintto own lint text + JSON rendering. - Updated
internal/cli/lint.goto render lint results via the new output adapter. - Converted
internal/reportinto a forwarding compatibility facade and added exact-output tests for the new renderer.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/report/text.go | Turns report.Text into a forwarding wrapper to the new output adapter. |
| internal/report/json.go | Adds compatibility-facade package docs and forwards report.JSON to the new output adapter. |
| internal/output/lint/lint.go | New canonical implementation of lint text and JSON rendering. |
| internal/output/lint/lint_test.go | Adds exact-output tests for the new renderer (and should include an import-path regression check). |
| internal/output/doc.go | Introduces the internal/output root package documentation. |
| internal/cli/lint.go | Switches lint CLI rendering to internal/output/lint for both text and JSON modes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86311b5432
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f26c3105b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Move lint text and JSON rendering into the layered
internal/output/lintpackage.Why
This establishes the output boundary described in Discussion #58 and keeps CLI orchestration separate from typed result rendering.
Closes #70.
Type
Breaking change
Changes
internal/output/lintfor lint text and JSON rendering.internal/reportinto a temporary forwarding compatibility facade.User-visible changes
None. Existing lint output, JSON schema, exit codes, fix markers, and output-file behavior are unchanged.
Validation
Commands run:
gofmt -w <touched Go files>make lintgo test ./...go run ./cmd/vaar lint ...Additional commands: