refactor: extract Executor struct, remove CommandSucceeded from channel#38
Merged
Conversation
The Event enum previously carried both watcher events (ChangeDetected) and build-completion signals (CommandSucceeded) on the same mpsc channel. Extract a dedicated Executor struct that owns the build thread and lets the main loop poll for completion via JoinHandle. This removes the conceptual dual-role of Event and eliminates the channel round-trip for lock-release signalling. - Introduce Executor with spawn/cancel/take_result methods - Rename Event to WatchEvent (only ChangeDetected remains) - Refactor run() to poll exec.take_result() instead of receiving CommandSucceeded through the watcher channel - No functional change
Only the Timeout branch poll is needed — builds that complete during the debounce wait are caught there. The outer-loop and inner-loop-top polls always see None.
yozhgoor
requested changes
Jun 22, 2026
- Remove generation from Executor — staleness handled by dropping JoinHandle on cancel(). take_result() only sees the current build. - Add WatchEvent::BuildFinished variant sent from build thread on exit. Wakes recv_timeout immediately so lock is released without waiting for the debounce timeout.
Executor no longer depends on mpsc. The wake-up mechanism is abstracted behind a plain callable.
This reverts commit f180e97.
yozhgoor
requested changes
Jun 23, 2026
yozhgoor
approved these changes
Jun 23, 2026
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.
The
Eventenum previously carried both watcher events (ChangeDetected)and build-completion signals (
CommandSucceeded) on the samempscchannel. Separate these concerns by introducing a dedicated
Executorstruct that owns the build thread and reports completion through the
channel via a lightweight
BuildFinishednudge.Changes
Executorstruct withspawn/cancel/take_result— encapsulatesSharedChild,JoinHandle<bool>, and a wake-up callbackWatchEvent(formerlyEvent) — onlyChangeDetected { git: bool }and
BuildFinishedremain;CommandSucceededis gonerun()— build completion is signaled by aBuildFinishednudge onthe channel, which wakes
recv_timeoutimmediately. The lock isreleased by polling
exec.take_result()in theBuildFinishedhandler.