Add grouped writable options configuration API - #104
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79bedb01cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caefbe24ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _options.Clear(); | ||
| _options.Add(optionBuilder.BuildOptions(instanceName)); |
There was a problem hiding this comment.
When an already initialized type is reinitialized and BuildOptions throws—for example, because the configured file provider rejects the selected directory—this clears the valid registration before the replacement has been built. The failed call therefore leaves the type uninitialized, so a subsequent WritableOptions.GetOptions<T>() throws instead of retaining the previous configuration; the internal grouped replacement path has the same ordering. Build the new configuration into a local before clearing and replacing the list.
Useful? React with 👍 / 👎.
Build replacement configurations before clearing existing registrations so failed initialization leaves valid options intact. Add coverage for direct and grouped initialization paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72eef30b19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var replace = registeredTypes.Add(typeof(T)); | ||
| WritableOptions.InitializeInternal(instanceName, builder, replace); |
There was a problem hiding this comment.
When a grouped static initialization contains multiple registrations for the same type and a later recipe fails during configuration or BuildOptions, the first recipe has already cleared the previously valid set and installed only itself. The fresh multi-registration path therefore still leaves partial state after an exception—for example, reinitializing two named instances where the second provider rejects its directory loses all old names and retains only the first replacement—despite the single-registration failure case now preserving prior configuration. Build the complete per-type replacement before mutating the shared instance.
Useful? React with 👍 / 👎.
1 participant
Summary
Testing