-
Couldn't load subscription status.
- Fork 25
feat: removed-feature-key-and-segment-key-from-schema #210
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?
Conversation
.gitmodules
Outdated
| path = tests/engine/engine-tests/engine-test-data | ||
| url = git@github.com:Flagsmith/engine-test-data.git | ||
| branch = v2.5.0 | ||
| branch = feat/remove-feature-key-fields |
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.
TODO: remove once merged
package.json
Outdated
| "generate-evaluation-result-types": "curl -o evaluation-result.json https://raw.githubusercontent.com/Flagsmith/flagsmith/chore%2Fevaluation-context0key/sdk/evaluation-result.json && npx json2ts -i evaluation-result.json -o flagsmith-engine/evaluation/evaluationResult/evaluationResult.types.ts && rm evaluation-result.json", | ||
| "generate-evaluation-context-types": "curl -o evaluation-context.json https://raw.githubusercontent.com/Flagsmith/flagsmith/chore%2Fevaluation-context0key/sdk/evaluation-context.json && npx json2ts -i evaluation-context.json -o flagsmith-engine/evaluation/evaluationContext/evaluationContext.types.ts && rm evaluation-context.json", |
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.
TODO: revert once merged
sdk/models.ts
Outdated
| if (!flagsmithId) { | ||
| const flagsmith_id = flag.metadata?.flagsmith_id; | ||
| if (!flagsmith_id) { | ||
| continue; |
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.
Should we skip returning the feature if it doesn’t have an ID?
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.
yes that's what we are doing, as the continue skips adding it to the map
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.
No, I am saying, why should we do that? The metadata/ID being none is likely a bug?
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.
Python SDK is throwing an error in that case.
FeatureContext.feature_keyand useFeatureContext.namefor override matchingSegmentResult.keyFeatureMetadata.flagsmithIdtoFeatureMetadata.flagsmith_idmetadata.flagsmith_idevaluation/models.ts