Skip to content

Conversation

@jozkee
Copy link
Member

@jozkee jozkee commented Jun 2, 2020

Description

Port of #36829 to preview6.

Description from original PR (click to view)

Address the following as it was detailed in #30820 (comment):

  • The naming changes for ReferenceHandling to resemble a class.
  • Exposure of the ReferenceResolver class.
  • Add ReferenceHandler<T> where T : ReferenceResolver to allow users to safely provide their own resolver implementation that will be instantiated on each call to (De)Serialize.
  • Make ReferenceHandler abstract so if you override it you can opt-in for using a persistent resolver.

This PR effectively closes #30820 as no more changes would be pending to address.

Customer Impact

This allow users to try out the latest shape of the ReferenceHandler API and give preview customers an early notice that their code using the feature needs to be updated since it is a breaking change between previews.

Risk

Code has been tested but it is a breaking change between previews.

…dler (dotnet#36829)

* Expose ReferenceResolver and rename ReferenceHandling to ReferenceHandler

* Address some feedback

* Address feedback

* Clean-up code

* Change messages in string.resx

* Add test for a badly implemented resolver

* Address feedback.
@jozkee jozkee requested review from Anipik and layomia June 2, 2020 07:44
@jozkee jozkee requested a review from steveharter as a code owner June 2, 2020 07:44
@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@jozkee jozkee added the Servicing-consider Issue for next servicing release review label Jun 2, 2020
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jun 4, 2020
@ericstj
Copy link
Member

ericstj commented Jun 4, 2020

@ajcvickers this will impact EF as I see you just added consumption of this API in dotnet/efcore#20827. Be on the lookout for the ingestion PR and fix up the test to use the new API. @jozkee can you help them with this?

@ajcvickers
Copy link
Contributor

ajcvickers commented Jun 4, 2020 via email

@jozkee jozkee merged commit 74c06a9 into dotnet:release/5.0-preview6 Jun 4, 2020
jozkee added a commit to dotnet/efcore that referenced this pull request Jun 4, 2020
Follow up to dotnet/runtime#37296 (comment)

The property and the type ReferenceHandling were recently renamed to Referencehandler on dotnet/runtime#36829

Above PR was also ported to dotnet/runtime preview6 branch under dotnet/runtime#37296 therefore once this is into master, it should be ported to release/5.0-preview6 branch as well.
@jozkee jozkee deleted the preview6/refhandler_1 branch December 3, 2020 20:06
@ghost ghost locked as resolved and limited conversation to collaborators Jan 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants