-
Notifications
You must be signed in to change notification settings - Fork 109
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
Change FeatureFlagService.isFeatureEnabled return value from Boolean to Flow<Boolean> #1703
Conversation
@@ -33,3 +36,6 @@ interface FeatureFlagService { | |||
*/ | |||
suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean | |||
} | |||
|
|||
suspend fun FeatureFlagService.isFeatureEnabled(feature: Feature): Boolean = |
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.
Adding a handy extension to avoid touching the code too much.
13a4fac
to
d6820d8
Compare
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
d6820d8
to
1e0dd49
Compare
override suspend fun isFeatureEnabled(feature: Feature): Boolean { | ||
return if (feature is FeatureFlags) { | ||
when(feature) { | ||
override suspend fun isFeatureEnabledFlow(feature: Feature): Flow<Boolean> { |
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.
suspend
here shall be removed
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.
Yes, updated.
@@ -33,3 +36,6 @@ interface FeatureFlagService { | |||
*/ | |||
suspend fun setFeatureEnabled(feature: Feature, enabled: Boolean): Boolean | |||
} | |||
|
|||
suspend fun FeatureFlagService.isFeatureEnabled(feature: Feature): Boolean = |
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 makes it a breaking change.
If you kept this API as it was and added the flow variant to the interface, it would be an "addition only change" and hence non-breaking.
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.
You're right, I have updated the PR.
01c9cd8
to
b5c68f1
Compare
Kudos, SonarCloud Quality Gate passed! |
Type of change
Content
Change FeatureFlagService.isFeatureEnabled return value from Boolean to Flow
Motivation and context
Be able to have "live" behavior of feature flag. Until now this was not necessary because when user goes to the settings to switch a flag, the flag is read again when entering the screen where the flag has an effect. Ex: Timeline.
But without this, it would be a bit harder (yet not impossible) to have a flag that have effect on the room list for instance, since the settings screen is a subscreen of the room list.
In particular, we will need this to be able to add a feature flag for the key backup. A banner can be displayed on the room list.
Screenshots / GIFs
NA
Tests
Tested devices
Checklist