Adding feature flags #372
Adding feature flags #372
Conversation
|
|
||
| public class FeatureFlagService : IFeatureFlagService | ||
| { | ||
| private readonly ISyncLocalStorageService _storage; // Injected storage service |
There was a problem hiding this comment.
ISyncLocalStorageService is used in a dedicated storage service, don't inject it directly here instead use the interface IClientStorage
| @@ -116,7 +116,29 @@ | |||
| </div> | |||
| </div> | |||
| </div> | |||
|
|
|||
| @if (selectedNetwork == "Angornet") | |||
There was a problem hiding this comment.
now I wonder if we actually want to only allow feature flags for testnet or also on mainnet.
There was a problem hiding this comment.
If we are keeping these as experimental features i think we should only allow on testnets,
I believe that feature flags for mainnet are more applicable when some of our 'stable' features start deviating from main purpose and not be relevant to most users
In that case we can allow the specific users to enable / disable those features
There was a problem hiding this comment.
If we really want the feature flags for both the mainnet and testnet, I think what we can do that while injecting the NetworkConfiguration, and then in it the method would return the dictionary of features depending on the network selected(current network)...
| { | ||
| return _storage.GetItem<Dictionary<string, bool>>("FeatureFlags") ?? new() | ||
| { | ||
| {"HW_Support", false} |
There was a problem hiding this comment.
hmm I am not a fan of putting our default feature flags in the storage class, it should come form the config file INetworkConfiguration
But I am not sure how we can pass it to here?
Perhaps inject the interface and return the feature flag collection from here?
There was a problem hiding this comment.
yeah that will be much better, on it
There was a problem hiding this comment.
@dangershony I have Implemented this in the latest commit
| @@ -29,5 +29,8 @@ public interface IClientStorage | |||
|
|
|||
| string GetCurrencyDisplaySetting(); | |||
| void SetCurrencyDisplaySetting(string setting); | |||
|
|
|||
| void setFeatureFlags(Dictionary<string, bool> featureFlags); | |||
There was a problem hiding this comment.
yes this is what I was looking for
| { | ||
| _featureFlagService.SetFeatureFlag(key, value); | ||
| _featureFlags[key] = value; | ||
| } |
There was a problem hiding this comment.
leave space between methods (fix it in a future pr)
Purpose:
Changes: