Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move the CreateCastingConverter<T>() method to the base JsonConverter type. #87211

Merged

Conversation

eiriktsarpalis
Copy link
Member

@eiriktsarpalis eiriktsarpalis commented Jun 7, 2023

This generic virtual method was necessary back when CastingConverter was strongly typed, but since #80755 got merged this is not strictly necessary so it can be refactored to something that is more code size friendly.

Contributes to #87078.

@ghost ghost assigned eiriktsarpalis Jun 7, 2023
@eiriktsarpalis eiriktsarpalis added size-reduction Issues impacting final app size primary for size sensitive workloads and removed area-System.Text.Json labels Jun 7, 2023
@ghost
Copy link

ghost commented Jun 7, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to #87078.

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json

Milestone: -

@ghost
Copy link

ghost commented Jun 7, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Contributes to #87078.

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

area-System.Text.Json, size-reduction

Milestone: -

Copy link
Member

@krwq krwq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming all tests pass

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-System.Text.Json size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants