-
Notifications
You must be signed in to change notification settings - Fork 34
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
Improving serialization #311
Conversation
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.
Thanks for starting the work here.
src/abstractions/serialization/KiotaJsonSerializer.Serialization.cs
Outdated
Show resolved
Hide resolved
src/abstractions/serialization/KiotaJsonSerializer.Serialization.cs
Outdated
Show resolved
Hide resolved
src/abstractions/serialization/KiotaSerializer.Serialization.cs
Outdated
Show resolved
Hide resolved
src/abstractions/serialization/KiotaJsonSerializer.Serialization.cs
Outdated
Show resolved
Hide resolved
src/abstractions/serialization/KiotaSerializer.Serialization.cs
Outdated
Show resolved
Hide resolved
It was a bit of pickle, but I think this is a pretty nice solution. Did I just notice that the backing store stuff if not referenced in any tests? |
src/abstractions/serialization/SerializationWriterProxyFactory.cs
Outdated
Show resolved
Hide resolved
src/abstractions/serialization/SerializationWriterFactoryRegistry.cs
Outdated
Show resolved
Hide resolved
This way you have to include these packages as well, they will throw otherwise.
I'm out of the office this week, and see that the formatting failed. I checked the box that I gave the contributors access to the branch. So if anybody wants to run |
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.
Thank you for making the changes!
@andrueastman for final review and merge.
I believe we'll need a version bump/changelog entry as well.
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.
Thanks for the contribution here @svrooij
Fixed #310