chore: remove dark-notify btop theme switcher#323
Conversation
dark-notify drove automatic light/dark btop theme switching via the toggle-btop-theme callback. Remove it entirely — all tools now use Catppuccin Mocha permanently. - Drop the cormacrelf/tap/dark-notify formula from the Brewfile - Remove the dark-notify LaunchAgent install block from bootstrap.sh (and the now-orphaned `local dotfiles_dir`) and its uninstall.sh teardown - Delete LaunchAgents/com.user.dark-notify.plist and home/.bin/toggle-btop-theme (the latter existed solely as dark-notify's callback) - Update README, CLAUDE.md, and the catppuccin-theming skill accordingly Also set Claude Code `tui: "fullscreen"` in settings.json. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
Summary
Clean, well-scoped removal of dark-notify and its toggle-btop-theme consumer. The deletion is consistent across Brewfile, bootstrap.sh, uninstall.sh, README, CLAUDE.md, the catppuccin-theming SKILL, and the LaunchAgent plist, and the diff confirms no dangling references. The orphaned local dotfiles_dir removal in install_launch_agents() is correct: it was used only by the deleted dark-notify block, and the install_launch_agent helper recomputes its own dotfiles_dir, so the cargo-sweep / obsidian-mcp / sysload installs are unaffected (and SC2034 is avoided). The bundled tui: fullscreen setting is disclosed in the PR body and out of scope for objection.
Findings
[Suggestion] bootstrap.sh (install_launch_agents) / uninstall.sh — migration on existing installs. Removing the install block means re-running bootstrap no longer manages the com.user.dark-notify LaunchAgent, but a machine that previously installed it still has the plist in its LaunchAgents folder loaded and running. After this change dark-notify keeps firing its -c callback against the now-deleted toggle-btop-theme (a dangling symlink in the bin dir), producing silent exec failures until manually torn down. uninstall.sh covers full teardown, but there is no migration path short of running it. Consider a one-shot launchctl bootout of the label plus rm -f of the plist in bootstrap so existing installs converge cleanly, or simply note the manual cleanup step in the PR. Harmless on a single machine — flagging for completeness.
Verdict
APPROVE — only a Suggestion; no Critical or Important issues.
(Note: inline comments were posted in-body here because the gh api inline path was not available in this environment.)
Automated review by Claude Code
Feedback AddressedSkipped (no PR change)
|
Summary
Removes
dark-notifyand its sole consumer (the btop theme toggle) from the setup.dark-notifydrove automatic light/dark btop theme switching via thetoggle-btop-themecallback; with it gone, all tools use Catppuccin Mocha permanently.Changes
Deleted
LaunchAgents/com.user.dark-notify.plist— the LaunchAgent runningdark-notifyhome/.bin/toggle-btop-theme— existed solely as dark-notify's callbackCode
Brewfile— drop thecormacrelf/tap/dark-notifyformulabootstrap.sh— remove the dark-notify install block ininstall_launch_agents(), plus the now-orphanedlocal dotfiles_dir(would trip shellcheck SC2034)uninstall.sh— remove the dark-notifylaunchctl bootout+ plist teardown (kept the independent btop theme-symlink cleanup)Docs
README.md— drop the optional btop dark-mode post-install step and dependency blockCLAUDE.md— remove the Dark Mode Switching component and dropcom.user.dark-notifyfrom the LaunchAgents listcatppuccin-theming/SKILL.md— rewrite the Dark/Light Mode section (all tools use Mocha permanently)Unrelated, bundled per request
home/.claude/settings.json— set Claude Codetui: "fullscreen"Testing
make lint✓make test✓make test-bootstrap✓ (39/39)grepconfirms zero remaining dark-notify references🤖 Generated with Claude Code