Fix enum serialization#61
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes enum serialization issues by replacing JsonPropertyName with JsonStringEnumMemberName for the Role and LoggingLevel enums to ensure their JSON output is in lower-case. It also removes the obsolete ToJsonValue extension method from the MCP client, simplifying the request creation for logging level changes.
- Updated the Role enum to use JsonStringEnumMemberName.
- Updated the LoggingLevel enum accordingly.
- Removed the ToJsonValue method from McpClientExtensions and updated its usage.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/ModelContextProtocol/Protocol/Types/Role.cs | Uses JsonStringEnumMemberName for lower-case serialization of Role. |
| src/ModelContextProtocol/Protocol/Types/LoggingLevel.cs | Uses JsonStringEnumMemberName for each enum value for consistency. |
| src/ModelContextProtocol/Client/McpClientExtensions.cs | Removes the obsolete ToJsonValue conversion in favor of direct enum usage. |
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
|
Thanks. Can you please add tests? |
5db3040 to
b9964e2
Compare
|
Added basic serialization tests for enums |
1b0c65d to
e729a1c
Compare
|
Added the |
|
Adding the |
Motivation and Context
Use
JsonStringEnumMemberNameforRoleandLoggingLevelenum to ensure their serialization is lower-case.Remove
McpClientExtensions.ToJsonValue(LoggingLevel)since serializing issue is fixed.How Has This Been Tested?
Tested in reproduction for #55.
Breaking Changes
N/A
Types of changes
Checklist
Additional context
N/A