NotificationsYou must be signed in to change notification settings
Fork5.2k
Star17.2k

Add DictionaryKeyPolicy support for EnumConverter [#47765]#54429

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

layomia merged 5 commits intodotnet:mainfromSkiFoD:skifod/issue-47765

Aug 3, 2021

Merged

Add DictionaryKeyPolicy support for EnumConverter [#47765]#54429

layomia merged 5 commits intodotnet:mainfromSkiFoD:skifod/issue-47765

Aug 3, 2021

Conversation

Copy link

Contributor

SkiFoD commentedJun 18, 2021•
edited
Loading

Sorry for the delay. I added the DictionaryKeyPolicy support for the EnumConverter + made two tests to cover the behavior.

Add DictionaryKeyPolicy support for EnumConverter [dotnet#47765]

a06986b

SkiFoD requested review fromeiriktsarpalis,jozkee,layomia andsteveharter ascode owners

June 18, 2021 18:59

ghost added the area-System.Text.Json label

Jun 18, 2021

Copy link

ghost commentedJun 18, 2021

Tagging subscribers to this area:@eiriktsarpalis,@layomia
See info inarea-owners.md if you want to be subscribed.

Issue Details

Author:	SkiFoD
Assignees:	-
Labels:	`area-System.Text.Json`
Milestone:	-

layomia reviewed

Jun 23, 2021

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs OutdatedShow resolvedHide resolved

Moved DictionaryKeyPolicy parsing code to WriteWithQuotes [dotnet#47765]

a6aebec

SkiFoD requested a review fromlayomia

July 13, 2021 10:20

terrajobst added the community-contributionIndicates that the PR has been added by a community member label

Jul 19, 2021

layomia reviewed

Jul 23, 2021

View reviewed changes

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs OutdatedShow resolvedHide resolved

layomia mentioned this pull request

Jul 23, 2021

Apply DictionaryKeyPolicy to enum keys (and other primitive key types?)#47765

Closed

Refactored the bugfix in accordance to the comments (dotnet#47765)

235c7ef

SkiFoD requested a review fromlayomia

July 27, 2021 15:02

layomia reviewed

Jul 27, 2021

View reviewed changes

Copy link

Contributor

layomia left a comment•
edited
Loading

Choose a reason for hiding this comment

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

Looks really close, just a few minor comments & the merge conflict needs to be resolved.

...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/EnumConverter.cs OutdatedShow resolvedHide resolved

...System.Text.Json.Tests/Serialization/CollectionTests/CollectionTests.Dictionary.KeyPolicy.cs OutdatedShow resolvedHide resolved

Sychev Vadim added2 commits

July 28, 2021 00:18

Made few minor corrections (dotnet#47765)

718fc71

Merge branch 'master' into skifod/issue-47765

d33dfdc

SkiFoD requested a review fromlayomia

July 27, 2021 21:26

karelz mentioned this pull request

Jul 28, 2021

[WinHttpHandler] Long running test: ResponseHeadersRead_SynchronizationContextNotUsedByHandler on Win7/Win8#54034

Closed

layomia approved these changes

Aug 3, 2021

View reviewed changes

Copy link

Contributor

layomia left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

layomia added the bug label

Aug 3, 2021

layomia merged commit814fb1b intodotnet:main

Aug 3, 2021

Copy link

ContributorAuthor

SkiFoD commentedAug 3, 2021

Thank you for the guidance through the issue. It's cool to be able to participate :).

Copy link

Member

danmoseley commentedAug 3, 2021•
edited
Loading

Thanks for contributing@SkiFoD ! There are many other JSON issues and if there of one you find interesting we would welcome another contribution from you!

thaystg added a commit to thaystg/runtime that referenced this pull request

Aug 3, 2021

Merge remote-tracking branch 'origin/main' into thays_fix_56249

c0e7fb2

* origin/main: (64 commits)  [wasm][debugger] Create test Inherited Properties (dotnet#56754)  Mark new test as incompatible with GC Mark4781_1GcStressIncompatible (dotnet#56739)  Ensure MetadataEnumResult is sufficiently updated by MetaDataImport::Enum (dotnet#56756)  [mono] Remove gdb xdebug and binary writer support, it hasn't worked in a while. (dotnet#56759)  Update windows-requirements.md (dotnet#56476)  Update doc and generic parameter name for JsonValue.GetValue (dotnet#56639)  [wasm][debugger] Inspect static class (dotnet#56740)  Fix stack overflow handling issue in GC stress (dotnet#56733)  Use ReflectionOnly as serialization mode in case dynamic code runtime feature is not supported (dotnet#56604)  Move Windows Compat pack to NuGet pack task (dotnet#56686)  Fix build error when building some packages (dotnet#56767)  Simplify JIT shutdown logic in crossgen2 (dotnet#56687)  Fix race in crossdac publishing with PGO (dotnet#56762)  Add DictionaryKeyPolicy support for EnumConverter [dotnet#47765] (dotnet#54429)  Use ComWrappers in some Marshal unit-tests and update platform metadata  (dotnet#56595)  Set `DisableImplicitNamespaceImports_Dotnet=true` to workaround sdk issue (dotnet#56744)  Make sure ServerGCHeapDetails is up to date (dotnet#56056)  [libraries] Reenable System.Diagnostics.DiagnosticSorce.Switches.Tests on mobile (dotnet#56737)  Disable failing arm64 win10 Graphics.FromHdc tests  (dotnet#56732)  Match xplat event source conditions (dotnet#56435)  ...

thaystg added a commit to thaystg/runtime that referenced this pull request

Aug 4, 2021

Merge remote-tracking branch 'origin/main' into thays_implement_debug…

33652d5

…ger_proxy_attribute* origin/main: (340 commits)  add RID for Debian 11 (dotnet#56789)  [wasm] [debugger] Skip thread static field (dotnet#56749)  Fix timeouts in coreroot_determinism test in GC stress mode (dotnet#56770)  Use File.OpenHandle in Socket.SendFile directly (dotnet#56777)  accept empty realm for digest auth (dotnet#56369) (dotnet#56455)  [wasm][debugger] Create test Inherited Properties (dotnet#56754)  Mark new test as incompatible with GC Mark4781_1GcStressIncompatible (dotnet#56739)  Ensure MetadataEnumResult is sufficiently updated by MetaDataImport::Enum (dotnet#56756)  [mono] Remove gdb xdebug and binary writer support, it hasn't worked in a while. (dotnet#56759)  Update windows-requirements.md (dotnet#56476)  Update doc and generic parameter name for JsonValue.GetValue (dotnet#56639)  [wasm][debugger] Inspect static class (dotnet#56740)  Fix stack overflow handling issue in GC stress (dotnet#56733)  Use ReflectionOnly as serialization mode in case dynamic code runtime feature is not supported (dotnet#56604)  Move Windows Compat pack to NuGet pack task (dotnet#56686)  Fix build error when building some packages (dotnet#56767)  Simplify JIT shutdown logic in crossgen2 (dotnet#56687)  Fix race in crossdac publishing with PGO (dotnet#56762)  Add DictionaryKeyPolicy support for EnumConverter [dotnet#47765] (dotnet#54429)  Use ComWrappers in some Marshal unit-tests and update platform metadata  (dotnet#56595)  ...