-
Notifications
You must be signed in to change notification settings - Fork 5.1k
STJ: Adding support for JsonNamingPolicyAttribute #117079
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
base: main
Are you sure you want to change the base?
STJ: Adding support for JsonNamingPolicyAttribute #117079
Conversation
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis |
cc: @eiriktsarpalis maybe you could take a quick look if the direction is right |
I don't think that would be necessary, the final computed name should simply be reflected in the |
@@ -52,14 +52,14 @@ | |||
<Project Path="tests/System.Text.Json.Tests/System.Text.Json.Tests.csproj" /> | |||
</Folder> | |||
<Folder Name="/tools/" /> | |||
<Folder Name="/tools/gen/"> | |||
<Folder Name="/tools/gen/" Id="5ad0b2ae-1660-5889-7060-a20f14d475b9"> |
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.
What's this?
JsonKnownNamingPolicy.SnakeCaseLower => JsonNamingPolicy.SnakeCaseLower, | ||
JsonKnownNamingPolicy.SnakeCaseUpper => JsonNamingPolicy.SnakeCaseUpper, | ||
JsonKnownNamingPolicy.KebabCaseLower => JsonNamingPolicy.KebabCaseLower, | ||
_ => JsonNamingPolicy.CamelCase, |
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 doesn't feel right to me, an invalid policy should probably result in no changes being made. It should probably also trigger a diagnostic if using the source generator.
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.
Your approach looks good to me, carry on :-)
Implements #108232
My first walk in this park, so some guidance is very much appreciated.
Not sure if I shouldn't add a JsonNamingPolicy property to
JsonTypeInfo
, might make some stuff cleaner.Still missing
JsonMetadataServices.Helpers.cs