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

Improve feature toggle API to actually use default value #1822

Merged
merged 4 commits into from
Mar 23, 2022

Conversation

aitorvs
Copy link
Collaborator

@aitorvs aitorvs commented Mar 22, 2022

Task/Issue URL: https://app.asana.com/0/414730916066338/1202001932298291/f

Description

The FeatureToggle API (implementation) currently doesn't make use of the defaultValues passed in. This PR improves that by making use of them.

It also modifies other semantics:

  • before the isFeatureEnabled would return null when a feature that didn't exist was passed in as a parameter
  • right now the call will throw IllegalArgumentException but that scenario would mean the develper forgot to actually add the code 🤷

Steps to test this PR

  • All privacy features work as expected (including their default values)
  • AppTP features are all disabled in INTERNAL builds, and respect the defaults in any other build
  • Smoke tests for remote config

@aitorvs
Copy link
Collaborator Author

aitorvs commented Mar 22, 2022

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@aitorvs aitorvs force-pushed the feature/aitor/feature_toggle_api branch 8 times, most recently from dc88b59 to a95ae1b Compare March 22, 2022 20:35
@aitorvs aitorvs force-pushed the feature/aitor/feature_toggle_api branch 3 times, most recently from edee6d8 to a336430 Compare March 22, 2022 23:35
@aitorvs aitorvs force-pushed the feature/aitor/feature_toggle_api branch from a336430 to 8fc57a4 Compare March 22, 2022 23:54
@aitorvs aitorvs marked this pull request as ready for review March 23, 2022 08:23
Copy link
Contributor

@marcosholgado marcosholgado left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT! Left a few nits.

@aitorvs aitorvs force-pushed the feature/aitor/feature_toggle_api branch 3 times, most recently from b823401 to 3e50004 Compare March 23, 2022 13:04
@aitorvs aitorvs force-pushed the feature/aitor/feature_toggle_api branch from 3e50004 to 2a05ee7 Compare March 23, 2022 13:17
@aitorvs aitorvs merged commit 465c04f into develop Mar 23, 2022
@aitorvs aitorvs deleted the feature/aitor/feature_toggle_api branch March 23, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants