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

Remove SerializationManager.Register method #2610

Closed
ReubenBond opened this issue Jan 19, 2017 · 3 comments · Fixed by #2941
Closed

Remove SerializationManager.Register method #2610

ReubenBond opened this issue Jan 19, 2017 · 3 comments · Fixed by #2941
Assignees
Milestone

Comments

@ReubenBond
Copy link
Member

Currently, when SerializationMananger is scanning types in FindSerializationInfo, it checks types for the [RegisterSerializer] attribute and calls their Register method if one is found.

Since Register is a static, parameterless method, implementations currently call into the static SerializationManager.Register method to register serialization delegates.

With SerializationManager becoming non-static, a change needs to be made here and I see two options:

  1. Remove [RegisterSerializer] and Register altogether and instead rely on [Serializer(typeof(TargetType))] attributes like we do in our generated code.
  2. Change the signature of the Register method so that it takes a parameter of type SerializationManager or some purpose-built interface like so:
public interface ISerializerRegistrar
{
    void Register(
        Type type,
        SerializationManager.DeepCopier copier,
        SerializationManager.Serializer serializer,
        SerializationManager.Deserializer deserializer);
}

I'm in favor of the 1st option. What do you think?

@jason-bragg
Copy link
Contributor

+1 for option 1. Seems very straight forward.

My preference here would be for the SerializationManager to be agnostic to how registration occurs. That is, there can be an assembly processor that finds serializers and registers them with the SerializationManager, a test harness that registers them, some configuration logic that registers them, or any number of other systems that register them, but the SerializationManager is independent of these methods. All these methods depend on the SerializationManager, but the SerializationManager itself is agnostic.

@galvesribeiro
Copy link
Member

I'm all for 1 but I also agree that the registration mechanism shouldn't matter for SerializationManager as @jason-bragg mentioned.

@ReubenBond
Copy link
Member Author

@galvesribeiro that can come later. This effectively removes SerializationManager out of the registration path, so registration can happen however we want.

@sergeybykov sergeybykov added this to the 1.5.0 milestone Jan 23, 2017
@sergeybykov sergeybykov modified the milestones: 1.5.0-beta, 1.5.0 Apr 5, 2017
@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

Successfully merging a pull request may close this issue.

4 participants