-
Notifications
You must be signed in to change notification settings - Fork 75
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
remove default value in Modular #1977
Conversation
return `"${param.restApiName}": options.${ | ||
param.clientName | ||
} ?? ${getQuotedValue(param)}`; |
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.
the question mark change is because of we now using getOptional which used to be getOptionalWithoutDefault and for OpenAI's case, they have a default value, they previously go into getOptionalWithDefault function.
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.
Compared with all the places that is using options to build the request body, this is the only places that doesn't have a question mark.
As options by literal means this parameter is optional, we should keep options?.
no matter if it has a default value or not. Besides, even if it does have a default value. options?.
won't report any errors.
@@ -456,7 +456,7 @@ export function _beginAzureBatchImageGenerationSend( | |||
...operationOptionsToRequestParameters(options), | |||
body: { | |||
prompt: prompt, | |||
n: options.n ?? 1, | |||
n: options?.n, |
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 is the change about question mark
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
for comment #1975 (comment)