-
Notifications
You must be signed in to change notification settings - Fork 23
fix: Ensure EvaluationContext is reliably added to the injected FeatureClient #605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Summary of ChangesHello @kylejuliandev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical bug in the OpenFeature .NET SDK's hosting integration that prevented the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively resolves an issue where the EvaluationContext was not reliably applied to an injected IFeatureClient if AddContext() was called after the client was configured. The fix correctly refactors the client registration by removing the state-dependent IsContextConfigured flag and instead resolving the EvaluationContext from the service provider when the IFeatureClient is requested. This makes the configuration order irrelevant and the behavior more robust. The associated tests have been updated, and a new test correctly validates the fix. The overall change is a significant improvement in correctness and code simplicity.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #605 +/- ##
==========================================
- Coverage 89.76% 89.73% -0.04%
==========================================
Files 77 77
Lines 3166 3156 -10
Branches 364 364
==========================================
- Hits 2842 2832 -10
Misses 253 253
Partials 71 71 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a small question. Other than that looks good to me.
* Removing this was technically a breaking change Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
This PR
Fixes an issue with the
EvaluationContextnot being added to the API FeatureClient if a call toAddContext()was made after adding the Provider. For example, the following would fail to inject theEvaluationContext:The fix is not ported to the old Dependency Injection code as we are actively deprecating it.
Related Issues
Fixes #542
Notes
Follow-up Tasks
How to test
Before
After