-
Notifications
You must be signed in to change notification settings - Fork 757
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
Use the same JsonSerializerOptions default in all locations. #5507
Conversation
/// <summary>Lazily-initialized default options instance.</summary> | ||
private static AIFunctionFactoryCreateOptions? _defaultOptions; | ||
|
||
/// <summary>Creates an <see cref="AIFunction"/> instance for a method, specified via a delegate.</summary> | ||
/// <param name="method">The method to be represented via the created <see cref="AIFunction"/>.</param> | ||
/// <returns>The created <see cref="AIFunction"/> for invoking <paramref name="method"/>.</returns> | ||
[RequiresUnreferencedCode(UsesReflectionJsonSerializerMessage)] |
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.
A number of these overloads are now redundant, but I defer to @stephentoub when and when they should be removed.
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.
Which are redundant / would you want to remove? Do you mean making the AIFunctionFactoryCreateOptions optional on the other overload?
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.
Correct.
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'd be ok if you wanted to consolidate them here. I'm more concerned about batching up breaking binary changes to the object model, as those are the things we'd expect nuget packages to be impacted by. AIFunctionFactory is less relevant there.
And removes all remaining RUC/RDC annotations from the codebase.
Microsoft Reviewers: Open in CodeFlow