-
Notifications
You must be signed in to change notification settings - Fork 8
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
fix: correct merging of contexts with targetingKey #136
Conversation
548d605
to
c1429a8
Compare
Signed-off-by: Tommaso Tofacchi <tofacchitommaso@gmail.com>
c1429a8
to
c2375d3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #136 +/- ##
============================================
+ Coverage 95.19% 95.71% +0.52%
Complexity 227 227
============================================
Files 40 40
Lines 583 584 +1
============================================
+ Hits 555 559 +4
+ Misses 28 25 -3 ☔ View full report in Codecov by Sentry. |
@beeme1mr shall I add a test hitting the line that is currently missing per |
I view it as optional for a change like this. |
Thank you for the PR @mmito 👋 This looks good to me. I will go ahead and override the codecov, since it is still a net improvement on coverage, and merge this in |
🤖 I have created a release *beep* *boop* --- ## [2.0.10](2.0.9...2.0.10) (2024-10-28) ### 🐛 Bug Fixes * correct merging of contexts with targetingKey ([#136](#136)) ([3f141d7](3f141d7)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This PR
Corrects the behavior of EvaluationContextMerger
merge()
when using stringtargetingKey
.Related Issues
Fixes #135
Notes
Follow-up Tasks
How to test
The
testEvaluationContextMergingTargetingKey
test in EvaluationContextTest.php has been added. The test fails without the proposed fix, as expected.