-
-
Notifications
You must be signed in to change notification settings - Fork 140
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
WIP Feature: Use Microsoft KeyedServiceProvider #1075
WIP Feature: Use Microsoft KeyedServiceProvider #1075
Conversation
…soft.Extensions.DependencyInjection adapter
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1075 +/- ##
==========================================
- Coverage 71.87% 71.79% -0.08%
==========================================
Files 96 96
Lines 4483 4429 -54
Branches 569 557 -12
==========================================
- Hits 3222 3180 -42
+ Misses 1074 1068 -6
+ Partials 187 181 -6 ☔ View full report in Codecov by Sentry. |
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.
Thanks for your efforts on this.
can we hold, until we have a think of the overall shape of this class. my heads been running through this and I think there's gaps in the unit tests around the use of MicrosoftDependencyResolver(IServiceProvider serviceProvider)
and Register
that aren't related to the PR itself.
if these need cleaning up it might make sense to make MicrosoftDependencyResolver(IServiceProvider serviceProvider)
become MicrosoftDependencyResolver(IKeyedServiceProvider serviceProvider)
to remove a bunch of the is
casts as well.
src/Splat.Microsoft.Extensions.DependencyInjection/MicrosoftDependencyResolver.cs
Show resolved
Hide resolved
src/Splat.Microsoft.Extensions.DependencyInjection/MicrosoftDependencyResolver.cs
Outdated
Show resolved
Hide resolved
src/Splat.Microsoft.Extensions.DependencyInjection/MicrosoftDependencyResolver.cs
Outdated
Show resolved
Hide resolved
@dpvreony In this case should I provide these test you have mentioned? |
leave it with me |
src/Splat.Microsoft.Extensions.DependencyInjection/MicrosoftDependencyResolver.cs
Show resolved
Hide resolved
src/Splat.Microsoft.Extensions.DependencyInjection/MicrosoftDependencyResolver.cs
Show resolved
Hide resolved
src/Splat.Microsoft.Extensions.DependencyInjection/MicrosoftDependencyResolver.cs
Show resolved
Hide resolved
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Use KeyedServiceProvider instead of ContractDictionary for Splat.Microsoft.Extensions.DependencyInjection adapter.
What is the current behavior?
Splat and ServiceProvider provides different instances of a singleton object
#1074
What is the new behavior?
Splat and ServiceProvider provides the same instance of a singleton object
What might this PR break?
Nothing known.
Please check if the PR fulfills these requirements
Other information: