-
-
Notifications
You must be signed in to change notification settings - Fork 658
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: draft branch playground evaluation #3967
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
environment: string; | ||
}; | ||
|
||
type AdvancedPlaygroundFeatureSchema = PlaygroundFeatureSchema & { |
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.
This will be openapi schema once we define it, but for now I neded to add extra context and environment info to prepare UI specific read model
@@ -42,34 +50,49 @@ export class PlaygroundService { | |||
projects: typeof ALL | string[], | |||
environments: string[], | |||
context: SdkContextSchema, | |||
): Promise<PlaygroundFeatureSchema[]> { | |||
): Promise<any> { |
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.
this type will be openapi defined in the next PR
entries, | ||
(entry) => entry.environment, | ||
); | ||
return { |
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.
it creates the same structure as we had in the hardcoded response
); | ||
return { | ||
name, | ||
projectId: entries[0]?.projectId, |
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.
we can take any entry in the group so I take the first one since it's always there
@@ -112,18 +135,22 @@ export class PlaygroundService { | |||
projectId: featureProject[feature.name], | |||
variant: client.getVariant(feature.name, clientContext), | |||
name: feature.name, | |||
environment, |
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'm passing environment and context from input to output because the new UI requires those values
context, | ||
environment, | ||
}); | ||
return result.map((item) => omitKeys(item, 'environment', 'context')); |
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.
old playground shouldn't get environment and context that we evaluate for the new playground
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.
LGTM
About the changes
Drafting batch playground/advanced evaluations instead of a hardcoded response.
Next PR:
Important files
Discussion points