Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move SerilogLoggerFactory to Serilog.Extensions.Logging #53

Closed
ChrisSimmons opened this issue Jul 1, 2018 · 5 comments · Fixed by #87
Closed

Move SerilogLoggerFactory to Serilog.Extensions.Logging #53

ChrisSimmons opened this issue Jul 1, 2018 · 5 comments · Fixed by #87

Comments

@ChrisSimmons
Copy link

From a distance, it seems that while SerilogWebHostBuilderExtensions deals with bona fide ASP.NET concerns (e.g IWebHostBuilder), SerilogLoggerFactory would be better placed in a more generic .NET Core library. Perhaps Serilog.Extensions.Logging.

There's also the possibility that I'm completely insane. :)

Of course if not, I realize this would be a breaking change but maybe something to consider in the next major version of this library.

@khellang
Copy link
Member

khellang commented Jul 1, 2018

It could be moved and type-forwarded, since this package already depends on Serilog.Extensions.Logging 😊

@nblumhardt nblumhardt changed the title Question: Why is SerilogLoggerFactory in this library? Move SerilogLoggerFactory to Serilog.Extensions.Logging Jul 1, 2018
@ChrisSimmons
Copy link
Author

I'll happily take this task (unless of course someone else comes along and does the hard work)

@jasper-d
Copy link

jasper-d commented Feb 8, 2019

@ChrisSimmons What's the status of this?

@nblumhardt
Copy link
Member

This is part of serilog/serilog-extensions-logging#132

@nblumhardt
Copy link
Member

@khellang RE the type forwarding, #87 doesn't do it as I took liberties breaking the SerilogLoggerFactory constructor's signature... Does seem like it would be nice and neat to type-forward here, though :-)

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants