Library: split di.go into six files and dedupe the hooks - #47
Merged
Conversation
di.go was 2447 lines, a third of them comments, with the instance lifecycle 1200 lines away from the Start and Stop that drive it. The library is now di.go (package doc, keys, events, Scope, modules, Test), binding.go (registration), state.go (the registry and freeze), resolve.go (the resolution path, both cycle detectors, the build step), lifecycle.go (the phase machine, hooks, Start and Stop) and run.go (Run and Shutdown). Code moved verbatim; the edits came after. Simplifications: one state.report helper replaces six hand-built event lines; Wire and Wrap share the signature check, parameter keys and argument loop; instance.paired and instance.owes are the one statement of the owed predicate for both the drain and stop steps; once.wait returns its error last; two unreachable branches are gone (a nil start context, an eager key missing from the index). One behaviour fix rode along. instance.start called OnStart directly rather than through callHook, so a panicking start hook emitted no EventStart while a panicking drain or stop hook emitted its event with the error. It now goes through callHook, which also removes the duplicate recover block in startClaimed. TestPanickingStartHookIsObserved pins it and fails against 80895d2 with "observed 0 start events, want 1". Comments: the history told as narrative, the five restatements of the no-Stop-from-a-hook rule and the comments that restated code are gone; the invariant comments and the public godoc stay. CLAUDE.md, docs/DESIGN.md and the explain.go header describe the new layout. Co-Authored-By: Claude Fable 5.1 <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
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.
Why
di.gowas 2447 lines with 842 of comment, and the instance lifecycle (start,stopIfNeeded,drainIfNeeded,stop) sat 1200 lines away from theStart,Stopanddrainthat drive it. This came out of a review of the file for simplification, comment bloat and organisation.The split
Code moved verbatim by line range first (that state builds and passes the suite), then the edits went on top.
di.goScope, modules,Test,WithScope/FromContextbinding.gobinding,Provide/Value/Wire/Wrap, theBindinghandlestate.gostate,freeze,deriveEager, the parent-chain readersresolve.goresolver,graph,get/resolve/await, the build step,Getand friendslifecycle.goonce,Start,Stop, the drain sweeprun.goRun,Shutdown, signalscallsitecounts frames, not files, andscripts/generatorgap.goalready indexes by file, so neither needed a change. CLAUDE.md,docs/DESIGN.mdand theexplain.goheader describe the layout.Simplifications
state.reportreplaces the six hand-builtemit(Event{...})lines.WireandWrapsharefunction(signature check, same case order and messages),paramsandScope.arguments.instance.pairedandinstance.owesare the one statement of the owed predicate, used by both the drain and stop steps.once.waitreturns(finished, err).startIfRunning(runContextnever reportsrunningwith a nil context) and thecontinueinderiveEagerfor a key absent from the index (every non-group binding inallis indexed). Both had zero coverage.constructbuilds its view with one literal.One behaviour fix
instance.startcalledOnStartdirectly rather than throughcallHook, so a panicking start hook emitted noEventStart, while a panicking drain or stop hook emitted its event with the error. It now goes throughcallHook, which also removes the duplicate recover block instartClaimed.TestPanickingStartHookIsObservedpins it and fails against a clone of80895d2withobserved 0 start events, want 1. CHANGELOG has an Unreleased entry; no signature changed.Comments
Removed: history told as narrative ("used to", "was how that showed"), the five restatements of the no-
Stop-from-a-hook rule (kept in full onStop), and comments that restated the code. Kept: every invariant comment and the public godoc. Net comment lines went from 842 to 798; the file headers and new helper docs added about 45 back.Left out on purpose: an
ancestors()iterator over the nine parent-chain walks, because it would sit on the warmlookup/isStoppedpath and should be benchmarked first.Verification
passes as one chain;
cd examples && go vet ./... && go test ./...passes;scripts/generatorgap.go -floor 90reports 93.1% locally. Benchmarks not run: the warmGetpath is untouched.🤖 Generated with Claude Code