-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Props - Remove usage of expressions #7577
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
base: dev
Are you sure you want to change the base?
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.
Still not made movement on the Create<TActor>(params object[] args) signature but interesting that so much is passing currently
| /// <exception cref="ArgumentException">This exception is thrown if <paramref name="type" /> is an unknown actor producer.</exception> | ||
| public Props(Deploy deploy, Type type, params object[] args) | ||
| #pragma warning disable CS0618 // Type or member is obsolete | ||
| : this(CreateProducer(type, args), deploy, args) // have to preserve the "CreateProducer" call here to preserve backwards compat with Akka.DI.Core |
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.
I have a feeling removing this is not supposed to happen. Still need to understand any test failures.
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.
I removed it from #7903 - it's a legacy function call that we probably don't need any more.
| /// </summary> | ||
| public static readonly Props None = null; | ||
| private Type _inputType; | ||
| private Type _outputType; |
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.
I expect this removal needs to come back in mainly due to the serialisation comment below on the TypeName property
| //for serialization | ||
| private set => _inputType = Type.GetType(value); | ||
| get => _producer.ActorType.AssemblyQualifiedName; | ||
| //private set => _producer.ActorType = Type.GetType(value); |
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.
Commented out for now
| var args = newExpression.Arguments.Count > 0 ? newExpression.GetArguments() : NoArgs; | ||
|
|
||
| return new Props(new ActivatorProducer(typeof(TActor), args), DefaultDeploy, args){ SupervisorStrategy = supervisorStrategy }; | ||
| return new Props(new FactoryProducer<TActor>(factory), DefaultDeploy, NoArgs) { SupervisorStrategy = supervisorStrategy }; |
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.
Func being used in the factory producer
| } | ||
|
|
||
| private class FactoryConsumer<TActor> : IIndirectActorProducer where TActor : ActorBase | ||
| private class FactoryProducer<TActor> : IIndirectActorProducer where TActor : ActorBase |
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.
Renamed to be more inline with the interface.
| public ActorBase Produce() | ||
| { | ||
| return Activator.CreateInstance(ActorType, _args).AsInstanceOf<ActorBase>(); | ||
| return (ActorBase)Activator.CreateInstance(ActorType, _args); |
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.
Not sure if there is a preference here.
| { | ||
| var copy = Copy(); | ||
| var original = copy.Deploy; | ||
| //var original = copy.Deploy; |
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.
Not needed as the todo below has commented it out.
I have a feeling that CreateProducer might still be needed but testing this out for now.
Moved some methods around to group better
Investigating how props can remove the usage of expressions as part of #7246