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

feat: Handle dependent feature toggles #218

Merged
merged 5 commits into from Oct 12, 2023
Merged

Conversation

chriswk
Copy link
Contributor

@chriswk chriswk commented Oct 12, 2023

  • A new feature added in beta to Unleash 5.6. With dependent feature toggle you can configure parent-child relationships. This change makes the SDK evaluate according to the client specifications defined for this.

strategy.getName());
}
// Dependent toggles, no point in evaluating child strategies if our dependencies are
// not enabled

Choose a reason for hiding this comment

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

not enabled -> not satisfied. satisfied may mean disabled or enabled depending on how we configure our dependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll update the comment

}
}
return new FeatureEvaluationResult(false, defaultVariant);
}
}

//
// Checks a feature's dependencies (if it has any

Choose a reason for hiding this comment

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

sth is off with the comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah. thanks

Copy link

@kwasniew kwasniew left a comment

Choose a reason for hiding this comment

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

Very nice. A few minor comments inside. 2 things I'd test since they are not part of the client specification:

  • metrics are not called on dependent features
  • impression events are called on dependent features for easier debugging

- A new feature added in beta to Unleash 5.6. With dependent feature toggle you
  can configure parent-child relationships. This change makes the SDK
  evaluate according to the client specifications defined for this.
@chriswk
Copy link
Contributor Author

chriswk commented Oct 12, 2023

Very nice. A few minor comments inside. 2 things I'd test since they are not part of the client specification:

  • metrics are not called on dependent features
    So we should not increment counts when checking parent
  • impression events are called on dependent features for easier debugging
    We should trigger impression events when checking parent.

Copy link
Contributor

@andreas-unleash andreas-unleash left a comment

Choose a reason for hiding this comment

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

LGTM

@chriswk chriswk merged commit f74e252 into main Oct 12, 2023
4 checks passed
@chriswk chriswk deleted the feat/dependentFeatures branch October 12, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants