-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Autorest Regen Preview to 3.0.0-alpha.20230712.2 by Dapeng Zhang from refs/pull/3373/merge #37553
Autorest Regen Preview to 3.0.0-alpha.20230712.2 by Dapeng Zhang from refs/pull/3373/merge #37553
Conversation
API change check APIView has identified API level changes in this PR and created following API reviews. |
a754e3b
to
dcc80f2
Compare
@@ -41,6 +41,6 @@ internal ChatChoice(ChatMessage message, int index, CompletionsFinishReason fini | |||
/// <summary> The ordered index associated with this chat completions choice. </summary> | |||
public int Index { get; } | |||
/// <summary> The reason that this chat completions choice completed its generated. </summary> | |||
public CompletionsFinishReason FinishReason { get; } | |||
public CompletionsFinishReason? FinishReason { get; } |
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.
Here is the corresponding typespec definition: https://github.com/Azure/azure-rest-api-specs/blob/982929d15d73166dbb9378e15c88b6e2ae5c4b80/specification/cognitiveservices/OpenAI.Inference/models/chat.completions.tsp#L196
This property is required and nullable, therefore this code change is expected.
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.
@joseharriaga does this mean Azure/autorest.csharp#3600 is fixed?
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.
or @ArcturusZhang is this part of the change in 3373?
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.
Looks like 3373 is fixing it, can we add fixes 3600
to the description on that PR
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 3373 fixes 3600
Close in favor of #37768 |
Triggered from Azure/autorest.csharp#3373