Add extension for getservice and replace usages #596
Conversation
| using System; | ||
| using Coverlet.Core.Attributes; | ||
|
|
||
| namespace coverlet.core.Extensions |
There was a problem hiding this comment.
nit: namespace casing Coverlet.Core.Extensions
| [ExcludeFromCoverage] | ||
| public static T GetService<T>(this IServiceProvider serviceProvider) | ||
| { | ||
| return (T)serviceProvider.GetService(typeof(T)); |
There was a problem hiding this comment.
Why [ExcludeFromCoverage]? core lib won't be instrumented so we don't need to exclude, you can remove it.
There was a problem hiding this comment.
Fixed, I added it as it was on HelperExtensions. Maybe if core lib won't be instrumented I should remove [ExcludeFromCoverage] from ``HelperExtensions` too?
There was a problem hiding this comment.
Ah ok...understood...I think it was placed to remove from "autocoverage" that we do on build.
| @@ -74,7 +75,10 @@ public void OnSessionStartShouldPrepareModulesForCoverage() | |||
| null, | |||
| _context); | |||
| IDictionary<string, object> sessionStartProperties = new Dictionary<string, object>(); | |||
| Coverage coverage = new Coverage("abc.dll", null, null, null, null, null, true, true, "abc.json", true, It.IsAny<ILogger>(), (IInstrumentationHelper)DependencyInjection.Current.GetService(typeof(IInstrumentationHelper)), (IFileSystem)DependencyInjection.Current.GetService(typeof(IFileSystem))); | |||
| Coverage coverage = new Coverage("abc.dll", null, null, null, null, null, true, true, "abc.json", true, | |||
There was a problem hiding this comment.
please keep declaration in one line...it's not so long at the moment, I prefer all in one line or all stacked.
There was a problem hiding this comment.
You are right consistency is better, fixed.
|
|
||
| namespace coverlet.core.Extensions | ||
| { | ||
| public static class DependencyInjectionExtensions |
There was a problem hiding this comment.
when we'll merge #566 we'll hide as internals...for now it's ok.
There was a problem hiding this comment.
Ok, for now it will stay public.
MarcoRossignoli
changed the title
Fix #554 this issue.
Add extension
GetService<T>to DependencyInjection for better readability and replaced all usages.