Skip to content
This repository was archived by the owner on May 30, 2024. It is now read-only.

Conversation

@ghost
Copy link

@ghost ghost commented May 17, 2016

So running off the tail of #50 - this is a slightly different approach which doesn't expose reps. Using something like this TestFeatureStore would allow users to configure the LDClient in a way that allows them to internally creates to be 100% "on" or "off" without any direct dependency on the rulesets coming from LD (e.g. configure it to also be .stream(false), etc).

This allows for runtime flipping of the flag values (with a reference to the configured TestFeatureStore) in test environments (e.g. perhaps where mocking is not viable), amongst other possibilities which don't want to depend on an external source of the flag rulesets. The primary desired comes from, for example, integration environments where rapidly changing the flag values via the API would be expensive.

The API / implementation is a bit rough and I want to open it up as is for discussion to see if this seems like a sensible way to go (especially with future incoming changes, etc) - or perhaps of a better alternative altogether?

}

private void writeFeatureRep(final String key, final Variation<Boolean> variation) {
FeatureRep<Boolean> newFeature = new FeatureRep.Builder<Boolean>(String.format("test-%s", key), key)
Copy link
Author

Choose a reason for hiding this comment

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

This is kind of hardcoded to Boolean based flags only at the moment...

@jkodumal
Copy link
Contributor

I think this will work pretty well. It will require changes with our new v2 API (and any time we make significant internal changes to FeatureRep) but it should be extremely easy to maintain as it does not leak internal representations.

So I'm on board with this approach.

@ghost
Copy link
Author

ghost commented May 18, 2016

Awesome, @jkodumal what further work would you recommend to get this over the line? Are you happy with the API, javadoc, etc?

@jkodumal
Copy link
Contributor

I think if we make the API a little more generic, we'll reduce or eliminate the need to make API-breaking changes to this class with v2. I'll add some comments where I think the API should be tweaked a bit.

@jkodumal
Copy link
Contributor

Actually, I think this will be fine. I think when v2 and multivariates come out we can extend this API and we won't necessarily need to modify it in breaking ways. I can't guarantee that though-- it might be the case that you'll need to make some minor tweaks to usages of this when v2 rolls out.

@jkodumal jkodumal merged commit 868d92a into launchdarkly:master May 18, 2016
eli-darkly added a commit that referenced this pull request Feb 21, 2018
make a few more classes package-private
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants