Adding netstandard 2.0 logging abstraction support #83
Conversation
|
Awesome, thanks Gert 👍 |
|
First time working with multi targeting csproj any tips would be helpful. |
|
May not have a chance to look at it today, but we've done a lot of multi-targeting in various Serilog projects, so if you hit any blockers, posting here should yield some tips :-) |
|
Will have to comeback to it later, it builds locally for me, not sure why appveyor is not finding the preview packages. |
gertjvr
changed the title
| /// <param name="logger">The Serilog logger; if not supplied, the static <see cref="Serilog.Log"/> will be used.</param> | ||
| /// <returns>The logger factory.</returns> | ||
| [EditorBrowsable(EditorBrowsableState.Never)] | ||
| public static IServiceCollection AddSerilog(this IServiceCollection services, ILogger logger) |
There was a problem hiding this comment.
This method can be removed - the reason for it appearing in SerilogLoggerFactoryExtensions is backwards binary compatibility.
| @@ -4,7 +4,7 @@ | |||
| <Description>Serilog provider for Microsoft.Extensions.Logging</Description> | |||
| <VersionPrefix>1.4.1</VersionPrefix> | |||
There was a problem hiding this comment.
I think we could rev this to 2.0.0 to signal the update supporting the new MEL.
|
Thanks a million for the PR, Gert :-) I'm still wrapping my head around some of the targeting requirements (the new logging packages still target netstandard1.1, for instance, so we don't strictly need to target netstandard2.0 AFAICT), but this will definitely get us started and seems the lowest-churn way forward 👍 |
|
We're in business :-) Thanks @gertjvr. @pakrym @davidfowl @BrennanConroy if anyone gets a change to kick the tyres/sanity check us, this package is out on NuGet as https://www.nuget.org/packages/Serilog.Extensions.Logging/2.0.0-dev-10164. Cheers! |
|
I am updating the sample project, will push as soon as I get it to build properly |
| /// logger is not specified but <paramref name="dispose"/> is true, the <see cref="Log.CloseAndFlush()"/> method will be | ||
| /// called on the static <see cref="Log"/> class instead.</param> | ||
| /// <returns>The logger factory.</returns> | ||
| public static IServiceCollection AddSerilog(this IServiceCollection services, Serilog.ILogger logger = null, bool dispose = false) |
There was a problem hiding this comment.
In the new world this extension method should go onto ILoggingBuilder
There was a problem hiding this comment.
Thanks @pakrym!
@gertjvr I just found the example at: https://github.com/aspnet/Logging/blob/dev/samples/SampleApp/Program.cs#L26
Are you still keen to tinker with this, or should we raise an issue for someone to pick up? Cheers!
|
Will have a look later today |
|
Maybe raise an issue so we can track it. |
This is ready to be reviewed!