-
Notifications
You must be signed in to change notification settings - Fork 74
Flags.enable and FlagsClient APIs #2900
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
Flags.enable and FlagsClient APIs #2900
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/feature-flagging #2900 +/- ##
============================================================
- Coverage 70.76% 70.72% -0.04%
============================================================
Files 834 834
Lines 30340 30378 +38
Branches 5128 5131 +3
============================================================
+ Hits 21469 21484 +15
- Misses 7415 7441 +26
+ Partials 1456 1453 -3
🚀 New features to boost your workflow:
|
…ent-manager-or-eliminate' into typo/FFL-1117-pass-default-client-configuration-to-flags-enable
…ent-manager-or-eliminate' into typo/FFL-1117-pass-default-client-configuration-to-flags-enable
…ent-manager-or-eliminate' into typo/FFL-1117-pass-default-client-configuration-to-flags-enable
argumentCaptor<FlagsFeature> { | ||
verify(mockSdkCore).registerFeature(capture()) | ||
// Verify the feature was created (implicit by successful registration) | ||
assertThat(lastValue).isNotNull |
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.
this assertion is kind of useless, because if registerFeature
was called, it means it was called with non-null value according to its signature which doesn't allow nullable type.
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.
updated to check the feature's name.
|
||
// When | ||
testedClient.setContext(fakeContext) | ||
testedClient.setEvaluationContext(fakeContext) |
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.
comment is regarding verify
below: instead of any
we could actually capture the argument is verify its properties.
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.
👍
...gs/src/test/kotlin/com/datadog/android/flags/featureflags/internal/DatadogFlagsClientTest.kt
Outdated
Show resolved
Hide resolved
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.
Resetting review status, because there is file which shouldn't be probably committed.
0349f3e
to
cdebda3
Compare
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.
alas, I have not been able to quite get through the merge conflicts from Jonathan's merges. tomorrow!
…lt-client-configuration-to-flags-enable
/merge |
View all feedbacks in Devflow UI.
The expected merge time in
|
Flags Feature: Enable method
Feature Flagging mobile SDK MVP spike
stacked on #2899
FFL-1117
FFL-1118
What does this PR do?
This PR implements the complete Flags feature initialization and client management APIs, bringing both
Flags.enable()
andFlagsClient
up to the mobile API specification. The PR includes configuration management, multi-domain client support, and graceful degradation patterns.Key Changes:
1. Flags.enable() Configuration
FlagsConfiguration
withcustomExposureEndpoint
FlagsFeature
to accept and storeFlagsConfiguration
for future client creationFlagsContext
and network layer2. FlagsClient Factory API
get()
method with multi-tenant support:get(name: String = "default", sdkCore: SdkCore)
setContext()
tosetEvaluationContext()
for OpenFeature alignment3. Multi-Client Management
unregister()
andclear()
methodsMotivation
This change fully implements the mobile API specification for Feature Flagging:
Review checklist (to be filled by reviewers)