Skip to content

Conversation

@simop-msft
Copy link
Contributor

@simop-msft simop-msft commented Mar 23, 2020

Fixes #3175

PR Type

What kind of change does this PR introduce?

  • Feature

What is the current behavior?

JsonConvert calls in BaseObjectStorageHelper always use default JsonSerializerSettings, and a caller cannot provide custom ones.

What is the new behavior?

A JsonSerializerSettings object can be provided to Local/RoamingObjectStorageHelper, and will then be used in all serialization and deserialization calls.

PR Checklist

Please check if your PR fulfills the following requirements:

@ghost
Copy link

ghost commented Mar 23, 2020

Thanks simop-msft for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@ghost ghost assigned Kyaa-dost Mar 23, 2020
@dnfclas
Copy link

dnfclas commented Mar 23, 2020

CLA assistant check
All CLA requirements met.

@dnfclas
Copy link

dnfclas commented Mar 23, 2020

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

❌ simop-msft sign now
You have signed the CLA already but the status is still pending? Let us recheck it.

vgromfeld
vgromfeld previously approved these changes Mar 23, 2020
Copy link
Contributor

@vgromfeld vgromfeld left a comment

Choose a reason for hiding this comment

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

:shipit:

@simop-msft simop-msft marked this pull request as ready for review March 23, 2020 16:43
@michael-hawker michael-hawker added this to the 6.1 milestone Mar 24, 2020
@simop-msft simop-msft force-pushed the user/simop/customSerializerSettings branch from a1e2126 to deb22db Compare March 25, 2020 10:14
@ghost
Copy link

ghost commented Mar 25, 2020

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

@simop-msft
Copy link
Contributor Author

Updated PR, but forgot to add header to new file IObjectSerializer. I also think that Microsoft.Toolkit.Uwp.Helpers is not necessarily a good place for this interface.

@simop-msft simop-msft force-pushed the user/simop/customSerializerSettings branch from deb22db to 54a2080 Compare March 25, 2020 13:58
@azchohfi azchohfi requested a review from vgromfeld March 25, 2020 18:42
Copy link
Contributor

@vgromfeld vgromfeld left a comment

Choose a reason for hiding this comment

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

:shipit:

@vgromfeld vgromfeld merged commit 87b0d06 into CommunityToolkit:master Mar 26, 2020
@michael-hawker michael-hawker mentioned this pull request Mar 27, 2020
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Allow providing a custom JsonSerializer to BaseObjectStorageHelper

7 participants