-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Adds support for NETCore to Akka.DI.Core and Akka.DI.TestKit #2732
Conversation
Unfortunately, there is no test for DI core, but I've tested it on Akka.DI.AutoFac mod for NetCore and tests are all passed under Core |
awesome, thanks @kantora - we'll review this shortly |
cc @heynickc can you review this? You're the expert on .NET Core assembly loading :p |
return firstTry ?? searchForType(); | ||
#elif CORECLR | ||
// I hope this will be enough as with core it is very hard to scan loaded assemblies. Although no internal method usages found | ||
return Type.GetType(typeName); |
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.
Might want to go ahead and add the searchForType()
fallback for CORECLR also by using a workaround that uses DependencyContext.Default.RunTimeLibraries
like:
private static Assembly[] GetAssemblies()
{
var assemblies = new List<Assembly>();
var dependencies = DependencyContext.Default.RuntimeLibraries;
foreach (var library in dependencies)
{
try
{
var assembly = Assembly.Load(new AssemblyName(library.Name));
assemblies.Add(assembly);
}
catch (Exception e)
{
//do nothing cant't if can't load assembly
}
}
return assemblies.ToArray();
}
@heynickc thanks for the help with code. I've updated the method. |
I've tested this only in Autofac tests. I hope today I can test it in some sandbox environment to tell fo sure. The Hyperion mod for Core just left to run it. |
Also, I can make a PR for Akka.Serialization.TestKit and Akka.Serialization.Hyperion for NETCore support, but it needs Hyperion 0.9.5 to be published in nuget. |
} | ||
} | ||
return assemblies; | ||
#else |
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'm thinking we don't need this #else
directive since there will only either be net45
or netstandard1.6
and if anyone were to add another target they would need to make sure the .csproj is updated correctly to cover either APPDOMAIN
or CORECLR
conditionals.
Ehm, Is something still needed from me to merge? |
@kantora I just kicked the tests off again for you. You should also rebase to the latest v1.3 branch changes. |
@heynickc done |
No description provided.