Update HttpClient logging APIs per Noah's comments #4469
Conversation
ghost
assigned 
geeknoid
force-pushed
the
geeknoid/renames
branch
from
429c3b3
to
001a5d5
Compare
geeknoid
force-pushed
the
geeknoid/renames
branch
from
001a5d5
to
23d8c13
Compare
| @@ -28,7 +28,7 @@ public static class HttpClientLoggingServiceCollectionExtensions | |||
| /// All other loggers are removed - including the default one, registered via <see cref="HttpClientBuilderExtensions.AddDefaultLogger(IHttpClientBuilder)"/>. | |||
| /// </remarks> | |||
| /// <exception cref="ArgumentNullException">Argument <paramref name="services"/> is <see langword="null"/>.</exception> | |||
| public static IServiceCollection AddDefaultHttpClientLogging(this IServiceCollection services) | |||
| public static IServiceCollection AddExtendedHttpClientLogging(this IServiceCollection services) | |||
There was a problem hiding this comment.
So, now both names are the same?
IServiceCollection.AddExtendedHttpClientLogging and IHttpClientBuilder.AddExtendedHttpClientLogging
Wouldn't it confuse users?
There was a problem hiding this comment.
IMO as long as serviceCollection.AddExtendedHttpClientLogging() has the same behavior as services.ConfigureHttpClientDefaults(httpBuilder => httpBuilder.AddExtendedHttpClientLogging()) then I wouldn't expect significant confusion. Worst case if a customer was aware of both and confused by it there is still no wrong answer, whichever one they try will work.
@davidfowl I think has stronger opinions that we should avoid offering IServiceCollection extension methods when a sub-builder extension method also exists. Personally I'm neutral about this pattern, but assuming I didn't misunderstand his position, he'd probably want us not to include the IServiceCollection variant.
There was a problem hiding this comment.
I'm happy. @davidfowl take a look since it seemed like you wanted to avoid extension methods simultaneously on IServiceCollection and sub-builders.
null
Microsoft Reviewers: Open in CodeFlow