Skip to content
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

Feature flags implementation #54

Merged
merged 6 commits into from
Jun 23, 2020
Merged

Conversation

TBG-FR
Copy link
Contributor

@TBG-FR TBG-FR commented Jun 19, 2020

Improved implementation of feature flags.
TLDR : All feature flags are now available through the plugin.

Modifications

  • Created a list of flags based on Jitsi Meet constants, as an Enum
  • Added flags descriptions (in comments) and values (in a Map)
  • Removed .setWelcomePageEnabled(false) from Kotlin code as it uses feature flags under the hood (see here)
  • Modified Android/Kotlin code to get and use the Map of feature flags instead of "static" fields
  • Modified iOS/Swift code to get and use the Map of feature flags instead of "static" fields
  • Modified example app to show how to use flags
  • Updated README with informations about feature flags

Why ?

  • We now have only an Enum and a Map to modify if new feature flags are added in Jitsi Meet SDK, we don't need to add attributes/fields and everything
  • Meeting options and feature flags are a bit more separated (no longer need one field in JitsiMeetOptions for each flag)
  • It's more convenient to use (developers just fill a map with flags they need to modify)
  • It's safer to use (thanks to the Enum and Map)
  • If no flags are provided, default values from the SDK will be used (we no longer need to put something with flagname ? : false and set an arbitrary true or false default setting

Reviews

I tested that modification under virtual devices, both on Android and iOS. If you want to test it yourself, go on, I'd be glad to get some feedback, and even maybe improvements 😉

@rwbr
Copy link
Contributor

rwbr commented Jun 22, 2020

I reviewed your changes and in my opinion everything looks fine. A test in the iOS-Simulator delivered a positive result also. I think, this PR should be accepted.

@TBG-FR
Copy link
Contributor Author

TBG-FR commented Jun 23, 2020

Thanks a lot for taking the time to review and test that, and your feeback 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants