Skip to content

Conversation

@rianjs
Copy link
Contributor

@rianjs rianjs commented Jan 29, 2026

Summary

  • Add RenderList, RenderKeyValue, RenderText, ListMeta, ListResponse to shared/view package
  • Update cfl commands to use shared view package
  • Update jtk to use shared view package via NewWithFormat
  • Delete tools/cfl/internal/view (227 lines + tests)
  • Delete tools/jtk/internal/view (131 lines + tests)

Impact

~895 lines removed through consolidation.

Breaking Changes

None - both CLIs maintain the same behavior with shared implementation.

Test plan

  • Build succeeds
  • All tests pass
  • Lint passes

Closes #16

🤖 Generated with Claude Code

@rianjs
Copy link
Contributor Author

rianjs commented Jan 29, 2026

Test Coverage Assessment

Summary

This PR consolidates duplicate view packages from cfl and jtk into the shared atlassian-go/view package. The test coverage for the new functionality is adequate but has gaps.

New Code Added (67 lines to shared/view/view.go)

Function Lines Tested? Notes
ListMeta struct 4 No direct test Used by ListResponse
ListResponse struct 4 No direct test Tested indirectly through RenderList
RenderList() 5 No Missing tests for JSON and table formats
renderListAsJSON() 19 No Internal function, should be tested via RenderList
RenderKeyValue() 12 No Missing tests for JSON, table, and noColor modes
RenderText() 3 No Missing test

Tests Deleted (612 lines)

The PR deletes test files from:

  • tools/cfl/internal/view/view_test.go (372 lines)
  • tools/jtk/internal/view/view_test.go (240 lines)

Critically, these deleted tests covered exactly the functions being added:

  • TestRenderer_RenderList_JSON_HasMore / TestRenderer_RenderList_JSON_NoMore / TestRenderer_RenderList_JSON_Empty
  • TestRenderer_RenderList_Table / TestRenderer_RenderList_Plain
  • TestRenderer_RenderKeyValue_Table / TestRenderer_RenderKeyValue_JSON
  • TestRenderer_RenderText

The Gap

The existing shared/view/view_test.go tests (424 lines) cover the original shared view functionality but not the newly added functions. The tests being deleted from cfl and jtk were covering the functionality that's now being moved to shared, but equivalent tests weren't added.

Recommendation

Add tests for the new functions to shared/view/view_test.go. At minimum:

  1. RenderList - Test JSON output with hasMore=true and hasMore=false, verify _meta structure
  2. RenderList - Test table format delegates properly (no _meta in output)
  3. RenderKeyValue - Test JSON format outputs valid JSON
  4. RenderKeyValue - Test table format with and without color
  5. RenderText - Basic output verification

The deleted tests in cfl and jtk provide a template - they should be migrated to the shared package, not just deleted.

Risk Assessment

Risk Level Reason
Behavior regression Low Both CLIs previously had passing tests for this code
Future regressions Medium No automated tests will catch bugs in RenderList or RenderKeyValue going forward

Verdict

The PR is functionally correct (it builds and existing tests pass), but it's incomplete from a test perspective. The PR description says "All tests pass" - this is technically true, but only because the tests that would fail don't exist in the shared package.

Suggested action: Add the missing tests before merging, or create a follow-up issue to track adding them.

- Add RenderList, RenderKeyValue, RenderText, ListMeta, ListResponse
  to shared/view package (from cfl's internal/view)
- Update cfl commands to use shared view package:
  - Replace NewRenderer with New
  - Replace RenderJSON with JSON
  - Replace renderer.Success(fmt.Sprintf(...)) with v.Success(...)
- Update jtk to use shared view package via NewWithFormat
- Delete tools/cfl/internal/view (227 lines)
- Delete tools/jtk/internal/view (131 lines + tests)

Both tools now use consistent view formatting from shared package.

Closes #16
@rianjs rianjs force-pushed the refactor/16-consolidate-view-packages branch from 02eefc2 to 9b613ea Compare January 29, 2026 20:14
@rianjs rianjs merged commit 68796d2 into main Jan 29, 2026
7 checks passed
@rianjs rianjs deleted the refactor/16-consolidate-view-packages branch January 29, 2026 20:15
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: consolidate view packages to shared

2 participants