Refactor timer to use LocalTimer interface for better testability#481
Open
Refactor timer to use LocalTimer interface for better testability#481
Conversation
Introduce the LocalTimer interface with StartTimer and StartBreakTimer methods, and implement it with ProcessLocalTimer (background OS processes). The internal startTimer/startBreakTimer functions now accept a LocalTimer parameter, enabling future alternative implementations to be injected. https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq
…elds Simplifies the interface by passing the full Configuration struct to StartTimer and StartBreakTimer, instead of extracting individual voice/notify fields at the call site. https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq
- Add WebTimer implementing the Timer interface via HTTP PUT requests - Add getTimers() as the single place that decides which timers are active (WebTimer when a room is configured, ProcessLocalTimer when TimerLocal=true, both can run simultaneously) - Simplify startTimer/startBreakTimer to iterate over the active timers - Move httpPutTimer/httpPutBreakTimer into webtimer.go - Wrap errors in timer implementations with descriptive messages https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq
- Move Timer interface, ProcessLocalTimer, WebTimer, GetTimers, RunTimer, RunBreakTimer into github.com/remotemobprogramming/mob/v5/timer - WebTimer now holds Room and TimerUser as struct fields (resolved by main before constructing the timer) - ExecuteCommandsInBackgroundProcess and VoiceCommand are exported from the timer package so main's moo() can use them - main/timer.go becomes a thin bridge: computes room/timerUser from git context, then delegates to timerpkg.RunTimer / timerpkg.RunBreakTimer - Remove executeCommandsInBackgroundProcess from mob.go (now in timer/) https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq
- Timer interface now has IsActive() bool, StartTimer(minutes int), StartBreakTimer(minutes int) — configuration no longer passed per call - ProcessLocalTimer and WebTimer receive config via constructor (NewProcessLocalTimer / NewWebTimer) - ProcessLocalTimer.IsActive() returns configuration.TimerLocal - WebTimer.IsActive() returns room != "" - GetTimers() constructs all implementations and filters by IsActive(), so each implementation decides itself whether it should run https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq
- NewWebTimer now takes only config.Configuration; room and timerUser are read from configuration.TimerRoom / configuration.TimerUser - GetTimers, RunTimer, RunBreakTimer drop the separate room/timerUser parameters — just configuration is passed through - main resolves the effective room (incl. WipBranchQualifier logic) and timerUser into the configuration copy before calling the timer package https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq
- timer/localtimer/ and timer/webtimer/ are now independent packages that register themselves via init() using timer.Register(Factory) - timer package no longer imports its implementations; dependency arrows are reversed: implementations depend on the timer package, not vice versa - timer.go (main) blank-imports both packages to trigger registration - mob.go uses localtimer.ExecuteCommandsInBackgroundProcess/VoiceCommand directly for the moo feature https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq
Timer implementations now control their activation by returning nil from their factory functions when not configured, eliminating the need for the IsActive() method in the Timer interface. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The getUserForMobTimer function is now encapsulated within WebTimer where it's actually used. The function now uses the git.Client instead of direct os/exec calls for better consistency with the codebase. Also includes refactoring: improved variable naming and string formatting in timer package. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…branch qualifier Refactors getMobTimerRoom() into two responsibilities: 1. enrichConfigurationWithBranchQualifier() in mob.go extracts the qualifier from the current WIP branch and writes it to the configuration 2. WebTimer constructor determines the effective timer room based on TimerRoomUseWipBranchQualifier setting This fixes the bug where the extracted qualifier was not written to the configuration, making it unavailable for other commands. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a
LocalTimerinterface to abstract the local timer functionality, enabling better testability and separation of concerns. The concrete implementation (ProcessLocalTimer) encapsulates the background process execution logic that was previously inline in the timer functions.Key Changes
New file
localtimer.go: IntroducesLocalTimerinterface with two methods:StartTimer(): Starts a regular timer with voice and notification supportStartBreakTimer(): Starts a break timer with voice and notification supportProcessLocalTimeras the default implementation using background OS processesModified
timer.go:startTimer()andstartBreakTimer()functions to accept aLocalTimerparameterexecuteCommandsInBackgroundProcess()calls with calls to theLocalTimerinterface methodsStartTimer()andStartBreakTimer()functions to instantiateProcessLocalTimer{}Modified
timer_test.go:ProcessLocalTimer{}as theLocalTimerparameterLocalTimerinterface for isolated testingImplementation Details
LocalTimerinterface allows for dependency injection, making it easy to provide mock implementations in testsProcessLocalTimermaintains the existing behavior by delegating to the same helper functions (getSleepCommand(),getVoiceCommand(),getNotifyCommand())https://claude.ai/code/session_01DTB35xCmhRgW5SQUsYAtAq