Conversation
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Clean, well-structured addition of a spinner utility and success/info helpers. Implementation is correct, well-tested, CI passes, and the code fits naturally into the existing _shared/ module.
Minor Observations
-
Misleading JSDoc in
spinner.ts:6: The comment"--json flag would be passed (NO_COLOR env var is set)"conflates two separate concerns. The--jsonflag detection happens inDashboardCommand.withSpinner()(which passessilent: true), not inisSilentMode(). TheNO_COLORenv var is a separate, independent condition. Consider rewording to:Explicitly requested via silent option (e.g., when --json flag is present). -
NO_COLORspec compliance (very minor): The NO_COLOR spec says the variable should be honored when present, regardless of value — including empty string. The current checkif (process.env.NO_COLOR)missesNO_COLOR="". A more spec-compliant check would beif ('NO_COLOR' in process.env). This is a very minor edge case unlikely to matter in practice. -
Unused import:
afterEachis imported but unused inspinner.test.ts— not flagged by biome, so no action needed.
Overall this is a clean PR. The spinner utility is well-designed with proper silent mode detection, the base class integration is minimal and non-intrusive, and the test coverage is thorough.
🕵️ claude-code · claude-opus-4-6 · run details
Summary
oraESM-native spinner package and createswithSpinner(message, fn)utility insrc/cli/dashboard/_shared/spinner.tsthis.success(msg)andthis.info(msg)helper methods onDashboardCommandbase class with colored emoji prefixes--jsonflag is present,NO_COLORenv var is set, orCIenv var is setChanges
package.json— addedoradependencysrc/cli/dashboard/_shared/spinner.ts(new) — exportswithSpinner(message, fn, options?)andisSilentMode(options?)utilitiessrc/cli/dashboard/_shared/base.ts— addedthis.success(msg),this.info(msg), andthis.withSpinner(message, fn)protected helpers onDashboardCommandtests/unit/cli/dashboard/spinner.test.ts(new) — unit tests forwithSpinner(success, failure, silent mode) andisSilentModetests/unit/cli/dashboard/base.test.ts— added tests forsuccess,info, andwithSpinnerhelpersTest plan
orapackage installed and importable in ESM contextwithSpinner(message, fn)shows animated spinner during asyncfnexecution and clears on success/failure--jsonflag is present,NO_COLOR=1, orCI=1DashboardCommand.success(msg)prints green ✓ prefixDashboardCommand.info(msg)prints blue ℹ prefixwithSpinner(success, failure, silent mode) andsuccess/infohelpers — all passCard
https://trello.com/c/69b845f8a51b096157ce85fa
🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details