Skip to content

Conversation

yasirfolio3
Copy link
Contributor

Summary

  • Introduction of OnDecision Notification Listener.
  • Implemented onDecisionListener in:
    activate/getVariation

Test plan

  • Unit tests implemented for activate/getVariation OnDecisionListener

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

LGTM

@msohailhussain msohailhussain requested review from aliabbasrizvi and a team April 3, 2019 05:45
@yasirfolio3 yasirfolio3 requested a review from jaeopt April 3, 2019 09:21
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

One more change needed

static let experiment = "experiment"
}

struct DecisionInfoKeys {
Copy link
Contributor

Choose a reason for hiding this comment

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

Spec uses "_" for all decision-info-keys instead of camel cases. Can you fix them all?

static let feature = "feature_key"
static let featureEnabled = "feature_enabled"
...

Copy link
Contributor

Choose a reason for hiding this comment

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

I see this camel-cases is acceptable and common to other SDKs as well.

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

LGTM

@aliabbasrizvi aliabbasrizvi merged commit 043d3f2 into master Apr 4, 2019
@aliabbasrizvi aliabbasrizvi deleted the yasir/ondecision-activate-getvariation branch April 4, 2019 18:06
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.

3 participants