Repository navigation
Add HostApplicationBuilder - #65109
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @dotnet/area-extensions-hosting Issue DetailsAddresses #61634. I'm on vacation starting tomorrow, so I'm going to leave this as a draft. I won't likely be able to make many changes to this PR until after I come back on the 17th, but please feel free to leave initial feedback.
|
| /// <item><description>enables scope validation on the dependency injection container when <see cref="IHostEnvironment.EnvironmentName"/> is 'Development'</description></item> | ||
| /// </list> | ||
| /// </remarks> | ||
| public static HostApplicationBuilder CreateApplicationBuilder() => new HostApplicationBuilder(); |
There was a problem hiding this comment.
How valuable are these methods? It feels wrong to have 2 public APIs that do exactly the same thing.
On the existing factory methods, We ConfigureDefaults over top of a new HostBuilder(). So for those it makes sense, since the ctor creates a blank builder, and the CreateDefaultBuilder creates one with certain things set.
But here, these factory APIs literally just call the ctor. So do we need both?
There was a problem hiding this comment.
During API review, I suggested adding just these static methods for discoverability. I agreed to add the equivalent constructors, but I think those are less important.
ae027f8 to
09f8891
Compare
|
@eerhardt @maryamariyan I think this is ready for another (hopefully final) review round. |
| } | ||
| } | ||
|
|
||
| internal sealed class CheckedServiceCollection : IServiceCollection |
There was a problem hiding this comment.
I wonder if this functionality shouldn't be added to ServiceCollection itself. That way people can just use ServiceCollection without having to define this wrapper in as many places as we do.
It is also in maui: https://github.com/dotnet/maui/blob/5913b3262e7f4c99c3a7fdeacac48037b730e72a/src/Core/src/Hosting/Internal/MauiApplicationServiceCollection.cs#L8-L23
See #44728 and #52363 for reasons to use ServiceCollection where possible.
There was a problem hiding this comment.
I like this idea. It would require adding new public API to ServiceCollection to trigger making it readonly. What do you think about a new instance method called MakeReadOnly()? I don't like adding an IsReadonly setter to the public API because that would allow transitioning back to not being readonly. If this makes sense, I can file a new API proposal. I don't think it should block this PR though.
There was a problem hiding this comment.
What do you think about an instance method call MakeReadOnly()?
I like that.
If this makes sense I can file a new API proposal.
Yes, please. If you have time.
I don't think it should block this PR though.
Agreed.
|
I addressed the new feedback and opened an issue for |
eerhardt
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the great work here @halter73.
@maryamariyan - any thoughts?
Addresses #61634.