Replace Options facade with ResilientFunctions Settings#12
Merged
Conversation
Options was a 1:1 mirror of ResilientFunctions' Settings (with dead, never-called Merge methods), so every Settings API change forced a parallel Options edit. Drop it and use Settings directly. - Delete Cleipnir/Options.cs - FlowsContainer/FlowsModule: accept and store Settings; WithOptions now takes Settings; pass it straight to FunctionsRegistry - FlowOptions.Merge now takes Settings - Tests and samples: construct Settings instead of Options - README: fix InitialEffect example to use EffectId (0.ToEffectId()) Settings' internal properties are already visible to the Cleipnir and Cleipnir.Tests assemblies via InternalsVisibleTo in the submodule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Default the settings parameter to null (falling back to new Settings()), so callers that don't customize settings can omit the argument entirely. Drop the now-redundant new Settings() at those call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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
Cleipnir.Flows.Optionshad become a 1:1 mirror ofCleipnir.ResilientFunctions.Domain.Settings— same parameters, same order — withMergehelpers that were never called. Every change to RF'sSettings(like theleaseLength→replicaHeartbeatFrequencyswap in #11) forced a parallel edit toOptions. This removes the duplicate and usesSettingsdirectly.This works because the RF submodule already exposes
Settings' internal properties to theCleipnirandCleipnir.Testsassemblies viaInternalsVisibleTo.Changes
Cleipnir/Options.cs.FlowsContainer— constructor andCreatenow takeSettings; passed straight intoFunctionsRegistry(no moreMapToSettings).FlowsModule—WithOptions(...)and the DI registration now useSettings.FlowOptions.Merge— now takesSettings.new Settings(...)instead ofnew Options(...)/Options.Default.InitialEffectexample to build anEffectId(0.ToEffectId()), matching the current API.Trade-off
This intentionally exposes RF's
Settingstype in theCleipnir.Flowspublic API (viaWithOptions). The previousOptionsfacade shielded consumers from that type, but since it mirroredSettingsexactly it provided no real abstraction — only maintenance overhead.FlowOptions(per-flow options) remains a distinct, smaller type and is unchanged in spirit.Testing
dotnet build Cleipnir.NET.slnsucceeds.dotnet test Cleipnir.Tests— 25/25 pass.🤖 Generated with Claude Code