-
Notifications
You must be signed in to change notification settings - Fork 6
Evaluate without impressions: add impressionsDisabled option to feature evaluation
#452
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: ss-preset
Are you sure you want to change the base?
Conversation
impressionsDisabled option to feature evaluation [WIP]
impressionsDisabled option to feature evaluation [WIP]impressionsDisabled option to feature evaluation
| return properties && Object.keys(properties).length > 0 ? { properties } : undefined; | ||
| let options = properties && Object.keys(properties).length > 0 ? { properties } : undefined; | ||
|
|
||
| const impressionsDisabled = maybeOptions.impressionsDisabled; |
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.
We would need to refactor the validateEvaluationOptions so that the impressionsDisabled option is only validated and used for the Evaluator, and for other SDKs it should be sanitized to false (or any other falsy value) to ignore the feature.
Eventually, if the feature is included in SDKs, the change can be rolled back.
| * | ||
| * @defaultValue `false` | ||
| */ | ||
| impressionsDisabled?: 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.
We should delete this TypeScript definition, if the feature is not supported yet for SDKs.
JavaScript commons library
What did you accomplish?
How do we test the changes introduced in this PR?
Extra Notes