refactor: replace global render registry with VerboseOutputFormatter interface#8
Conversation
…interface Replace the global mutable render registry (render_registry.go) with a VerboseOutputFormatter interface that checks can optionally implement. This eliminates global state and init-time side effects, making the rendering extensible through Go's standard interface mechanism. Key changes: - Remove render_registry.go and its global maps/mutex - Add VerboseOutputFormatter interface and DefaultVerboseFormatter - Simplify outputImpactedObjects to render per-check instead of aggregating by group/kind/checkType - Implement VerboseOutputFormatter on ImpactedWorkloadsCheck - Remove unused maxDisplay truncation logic from notebook renderer Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThis PR removes the render registry pattern for customizable impacted object rendering and replaces it with an interface-based verbose formatter approach. Checks can now implement VerboseOutputFormatter to provide custom output formatting, while a new DefaultVerboseFormatter handles standard rendering. Changes
Sequence DiagramsequenceDiagram
participant Cmd as Lint Command
participant Check as Check Instance
participant Formatter as Verbose Formatter
participant DR as DiagnosticResult
participant Out as Output Buffer
Cmd->>Cmd: Iterate over checks with impacted objects
Cmd->>Check: Type assert to VerboseOutputFormatter
alt Check implements VerboseOutputFormatter
Cmd->>Check: FormatVerboseOutput(out, dr)
Check->>DR: Read ImpactedObjects
Check->>Out: Write custom formatted output
else Check does not implement interface
Cmd->>Formatter: Create DefaultVerboseFormatter
Cmd->>Formatter: FormatVerboseOutput(out, dr)
Formatter->>DR: Read ImpactedObjects
Formatter->>Formatter: Group by namespace
Formatter->>Out: Write formatted output
end
Formatter->>Out: Return formatted impacted objects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
@andyatmiami changes a little bit the verbose output renderer as my original implementation was, well, clunky. It should preserve your output formatting for notebooks but having some additional review from your would be very appreciated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/lint/checks/workloads/notebook/impacted.go (1)
3-6: Consider using the standardioimport name for consistency.The
iopackage is aliased asiolibhere (line 6), while theVerboseOutputFormatterinterface inverbose_formatter.gouses the standardioimport. This works correctly since both resolve to the sameio.Writertype, but it's a minor inconsistency. If the alias exists to avoid a conflict with anotherioidentifier in this file, a comment would help; otherwise, aligning with the standard name improves readability.
andyatmiami
left a comment
andyatmiami
left a comment
There was a problem hiding this comment.
/lgtm
- more idiomatic go pattern
- no loss of functionality while reducing lines of code
-
notebooksimpacted-objectcheck still renders appropriately
Great improvement - no notes - thanks!
3 participants
Replace the global mutable render registry (render_registry.go) with a
VerboseOutputFormatter interface that checks can optionally implement.
This eliminates global state and init-time side effects, making the
rendering extensible through Go's standard interface mechanism.
Key changes:
aggregating by group/kind/checkType
Co-authored-by: Cursor cursoragent@cursor.com
Description
How Has This Been Tested?
Merge criteria:
Summary by CodeRabbit