-
Notifications
You must be signed in to change notification settings - Fork 19
Adds feature flags with an initial listen mode flag. #583
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
base: chore/update-kotlin-room
Are you sure you want to change the base?
Adds feature flags with an initial listen mode flag. #583
Conversation
Ladybug bundles JDK 21, which requires a Kotlin version bump, which in-turn requires a Room bump. No functional changes.
| class RuntimeFlag( | ||
| private val sharedPreferences: IVocableSharedPreferences, | ||
| private val key: String, | ||
| private val defaultValue: 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.
The patterns for defining flags and accessing their state feels a little off here.
Imagine I'm writing code in some biz logic somewhere. If I want to check a runtime flag, it seems like I'd do something like this (I just borrowed it from your test):
FeatureFlags.RuntimeFlag(sharedPrefs, "test_flag", true).enabled
The onus is on me, random hunk of biz logic, to know the specific key for the feature and what its default should be. It seems like both of those ideas could be baked into some kind of sealed/enum-y type that defines individual flags and that could be passed to a thing that determines the state for you.
Something like...
enum class RuntimeFlag(key: String, default:Boolean) {
LISTEN_MODE("listen_mode", false)
}
and then break the thing that resolves the flag out...
class RuntimeFlags(/* yadda, Shared Prefs probably */) {
operator fun get(flag: RuntimeFlag): Boolean = /* yadda, read from disk */
}
and in situ... (I got cute with the indexing syntax operator, that's optional)
val runtimeFlags = RuntimeFlags(/* yadda */)
val isMyFeatureEnabled = runtimeFlags[RuntimeFlag.LISTEN_MODE]
| * Build-time flags that are controlled via build configuration. | ||
| */ | ||
| object BuildTimeFlag : Flag { | ||
| override val enabled: Boolean = BuildConfig.ENABLE_LISTEN_MODE |
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.
How does this scale when there's more than one build-time flag?
581d4d6 to
e415ab8
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Description: