-
Notifications
You must be signed in to change notification settings - Fork 17
Spec-update-1 #42
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: main
Are you sure you want to change the base?
Spec-update-1 #42
Conversation
This is primarily useful for developing new versions of the API client - we can do a strict build where the Unleash client specification is expected to have nothing unparsable. Some users may wish to prevent silent failures when they upgrade and use new features though, so this could be useful there too.
This will prevent silent merging of API conformance updates that don't actually implement everything.
No unit tests yet; not sure if we need them or not, but the client spec is super thin on this.
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.
So I think there's stuff missing from this (and it's also 100% spec compliant, which tells me that we really need to overhaul those spec tests). It looks like the calculation to resolve stickiness is correct but not the work to resolve the stickiness field. In the spec tests, that always seems to be customField
but that's a coincidence - that field can actually be a custom field i.e. anything really.
In words, the feature works like this:
- Stickiness is no longer limited to random, userId, sessionId or default, it can now be anything
- In the case where random, userId, sessionId or default is not matched (e.g. stickiness is 'customField' or 'programmingLanguage' or anything really), the stickiness should search the context object for a property of that name and use the value from there
Depending on how you feel about Ruby, it might be more clear to see what the feature does from the PR that implements it there: https://github.com/Unleash/unleash-client-ruby/pull/69/files
so thats a super interesting choice. If I was designing the API I would absolutely not have put this in the same value location in the API as the enumeration values: because there is now a free-form string field where some values behave differently. e.g. 'default' or 'random' do not refer to context.properties['random']. Whats the right way to get review of such things into the unleash roadmap? |
No unit tests yet; not sure if we need them or not, but the client spec
is super thin on this.