Skip to content
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

[Breaking Change] Improve serializer registration #2591

Closed
ReubenBond opened this issue Jan 13, 2017 · 2 comments
Closed

[Breaking Change] Improve serializer registration #2591

ReubenBond opened this issue Jan 13, 2017 · 2 comments
Assignees
Milestone

Comments

@ReubenBond
Copy link
Member

ReubenBond commented Jan 13, 2017

Since we will not be supporting static SerialzationManager.Register(...) method, we need to change the signature of the method used to register custom serializers so that it takes some kind of context (eg, ISerializerRegistrar) so that serializers can be registered.

In a local branch, I've made the following changes:

  • [Serializer(typeof(TargetType))] automatically registers the type it's attached to as the serializer for TargetType. This includes when TargetType is an open generic.
  • Generated serializers no longer have Register methods or the associated attribute.
  • No additional class is generated for generic types since the Register method is no longer needed. i.e, no "master registerer" class.
  • The Register method in types marked with [RegisterSerializer] must accept a parameter of type ISerializerRegistrar.

So [RegisterSerializer] is rarely required. The one place it's required in our codebase is for the tests which register the same serializer for multiple different JSON types (JObject, JArray, etc) but that could just as easily be accomplished using an IExternalSerializer implementation. We could even remove [RegisterSerializer] altogether, but I'm not suggesting that yet.

Does this all sound fine?

Related to #467

EDIT: oh, and the [RegisterSerializer] change is a good candidate for something to add to a code analyzer

@ReubenBond ReubenBond self-assigned this Jan 13, 2017
@sergeybykov sergeybykov added this to the 1.5.0 milestone Jan 14, 2017
@ReubenBond
Copy link
Member Author

This was implemented in #2611, which has been merged. Eventually the Register method can be removed, but it's simply marked as obsolete for now.

@ReubenBond
Copy link
Member Author

ReubenBond commented Jan 27, 2017

This is a 'breaking change', as the title says, but it's not a very breaking change and only affects a small number of users and has a simple fix.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants