-
Notifications
You must be signed in to change notification settings - Fork 14
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: standalone targeting, fractional shorthand #168
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,6 +4,11 @@ | |||||||||||||||||||||
"title": "flagd Targeting", | ||||||||||||||||||||||
"description": "Defines targeting logic for flagd; a extension of JSONLogic, including purpose-built feature-flagging operations.", | ||||||||||||||||||||||
"type": "object", | ||||||||||||||||||||||
"properties": { | ||||||||||||||||||||||
"targeting": { | ||||||||||||||||||||||
"$ref": "#/definitions/targeting" | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}, | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh you're totally right. I've corrected this and updated all the tests! |
||||||||||||||||||||||
"definitions": { | ||||||||||||||||||||||
"targeting": { | ||||||||||||||||||||||
"title": "Targeting", | ||||||||||||||||||||||
|
@@ -462,7 +467,7 @@ | |||||||||||||||||||||
"$comment": "if we remove the \"sum to 100\" restriction, update the descriptions below!", | ||||||||||||||||||||||
"description": "Distribution for all possible variants, with their associated weighting out of 100.", | ||||||||||||||||||||||
"type": "array", | ||||||||||||||||||||||
"minItems": 2, | ||||||||||||||||||||||
"minItems": 1, | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This allows for the fractional shorthand support (no wieght == weight 1) |
||||||||||||||||||||||
"maxItems": 2, | ||||||||||||||||||||||
"items": [ | ||||||||||||||||||||||
{ | ||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,9 @@ title: flagd Targeting | |
description: Defines targeting logic for flagd; a extension of JSONLogic, including | ||
purpose-built feature-flagging operations. | ||
type: object | ||
properties: | ||
targeting: | ||
$ref: "#/definitions/targeting" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This allows for the use of targeting by itself. |
||
definitions: | ||
targeting: | ||
title: Targeting | ||
|
@@ -363,7 +366,7 @@ definitions: | |
description: Distribution for all possible variants, with their associated weighting | ||
out of 100. | ||
type: array | ||
minItems: 2 | ||
minItems: 1 | ||
maxItems: 2 | ||
items: | ||
- description: If this bucket is randomly selected, this string is used to as | ||
|
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
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.
i do not think this is actually what we want. this validates an object with a property targeting like
see https://www.jsonschemavalidator.net/s/yRS6TJ9g -
but what we actually want is a validation without the targeting property like in proposed in #165 (comment) something like
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.
Absolutely correct, thanks a lot.
I've made this change.