feat: restore functional option pattern for New() #28
No reviewers
Labels
No labels
needs-review
needs-review
needs-review
needs-review
needs-review
needs-review
needs-review
athena
athena-gemini
audit
clotho
clotho-gemini
codex
darbs-claude
security
wiki
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: core/go#28
Loading…
Add table
Reference in a new issue
No description provided.
Delete branch "feat/service-options"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
New()returnsResult, acceptsCoreOptionfunctionalsWithService(factory func(*Core) Result)— service factory receives Core during constructionWithOptions(Options)— key-value configurationWithServiceLock()— immutable conclave after constructionRestores the v0.3.3 service registration contract adapted for the dev branch DTO pattern. Services registered in
New()form the application conclave with shared IPC access. Each Core instance has its own bus scope.Test plan
go build ./...passesgo test ./...passes (all 24 files updated)TestNew_WithService_Good,TestNew_WithServiceLock_Good🤖 Generated with Claude Code
Co-Authored-By: Virgil virgil@lethean.io
- HandleIPCEvents only auto-registered for services the factory didn't register itself (prevents double handler registration) - Auto-discovery only creates Service{} placeholder when factory didn't call c.Service() — factories that register themselves keep full lifecycle Addresses Codex review findings 1 and 2 from third pass. Co-Authored-By: Virgil <virgil@lethean.io>Options is now a proper struct with New(), Set(), Get(), typed accessors. Result gains New(), Result(), Get() methods on the struct. WithOption("key", value) convenience for core.New(). options_test.go: 22 tests passing against the new contract. Other test files mechanically updated for compilation. Co-Authored-By: Virgil <virgil@lethean.io>Cli{}.New(c) replaces &Cli{core: c} in contract.go. 9 tests passing. Co-Authored-By: Virgil <virgil@lethean.io>- WithService now calls factory, discovers service name from package path via reflect/runtime (last path segment, _test suffix stripped, lowercased), and calls RegisterService — which handles Startable/Stoppable/HandleIPCEvents - If factory returns nil Value (self-registered), WithService returns OK without a second registration - Add contract_test.go with _Good/_Bad tests covering all three code paths - Fix core.go Cli() accessor: use ServiceFor[*Cli](c, "cli") (was cli.New()) - Fix pre-existing })) → }}) syntax errors in command_test, service_test, lock_test - Fix pre-existing Options{...} → NewOptions(...) in core_test, data_test, drive_test, i18n_test (Options is a struct, not a slice) Co-Authored-By: Virgil <virgil@lethean.io>Root cause: Result.New didn't mark single-value results as OK=true, breaking Mount/ReadDir/fs helpers that used Result{}.New(value, err). Also: data_test.go and embed_test.go updated for Options struct, doc comments updated across data.go, drive.go, command.go, contract.go. All tests green. Coverage 82.2%. Co-Authored-By: Virgil <virgil@lethean.io>d2412459f5to5362a9965c