Skip to content
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

Added extensions on IServiceCollection #70

Merged
merged 4 commits into from
Mar 21, 2023
Merged

Conversation

mburumaxwell
Copy link
Contributor

@mburumaxwell mburumaxwell commented Mar 18, 2023

This adds new extensions on IServiceCollection similar to those on IHostBuilder. Also changed the implementation of the ones in IHostBuilder to rely on the new extensions for easier maintenance in the future. The implementation logic as pertains to registration of services remains the same.

Fixes: #66, #68

mburumaxwell and others added 3 commits March 18, 2023 09:53
…ons.cs

Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>
Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>
…ExtensionsTests.cs

Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>
@nblumhardt nblumhardt merged commit df131bf into serilog:dev Mar 21, 2023
@nblumhardt
Copy link
Member

LGTM! Thanks @mburumaxwell and @sungam3r for the review 👍

I'll bump the dev package version to 5.1.0-* to indicate new feature additions.

@AraHaan
Copy link

AraHaan commented Mar 21, 2023

Can a followup pr be made to split the servicecollection versions over into an Serilog.Extensions.DependencyInjection library so that way those who do not want to use the hosting packages could get the benefits of using the new extensions?

Also, then the hosting package could depend on that package as well which should be ok as Microsoft.Extensions.DependencyInjection would be provided anyways.

@mburumaxwell
Copy link
Contributor Author

LGTM! Thanks @mburumaxwell and @sungam3r for the review 👍

I'll bump the dev package version to 5.1.0-* to indicate new feature additions.

Cool. Looking forward to the release.

@mburumaxwell
Copy link
Contributor Author

Can a followup pr be made to split the servicecollection versions over into an Serilog.Extensions.DependencyInjection library so that way those who do not want to use the hosting packages could get the benefits of using the new extensions?

Also, then the hosting package could depend on that package as well which should be ok as Microsoft.Extensions.DependencyInjection would be provided anyways.

This makes sense. Maybe you can do the PR?

If we have to move things, instead of a new package, I'd move the extensions to Serilog.Extensions.Logging (lives in a different repository) which is referenced by the hosting one here. In such a case, I wonder if the hosting package would still be needed seeing that one can do it. Maybe that's a different discussion?

@AraHaan
Copy link

AraHaan commented Mar 22, 2023

Yeah, I think putting it in the logging package would be better as well. And yes, I think the hosting package can be deprecated after this (which would help you guys more with less packages to maintain).

@mburumaxwell
Copy link
Contributor Author

@AraHaan I believe that's for @nblumhardt to decide, being the maintainer.

@AraHaan
Copy link

AraHaan commented Mar 25, 2023

Yeah, I know that. I was just saying what I was thinking as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add IServiceCollection extension methods to enable serilog
4 participants