NLog + log4net added NetStandard support #167
Conversation
|
|
snakefoot
force-pushed
the
develop
branch
4 times, most recently
from
514e768
to
2d5e2b1
Compare
| @@ -41,9 +41,9 @@ | |||
| <PrivateAssets>All</PrivateAssets> | |||
| </PackageReference> | |||
| <PackageReference Include="Microsoft.ApplicationInsights" Version="2.6.0-beta2" /> | |||
| <PackageReference Include="log4net" Version="2.0.5" /> | |||
| <PackageReference Include="log4net" Version="2.0.8" /> | |||
There was a problem hiding this comment.
is this update required for the PR? Can the older version reference be kept for better compatibility?
There was a problem hiding this comment.
Oh, I see... older version doesn't have netstandard target.
Can you please enable optional reference here - older version for net45 and newer fro netstandard.
There was a problem hiding this comment.
Now only using ver. 2.0.8 for netstandard, but I have to force log4net from 2.0.5 to 2.0.6 on net45 to support flush.
src/NLogTarget/NLogTarget.csproj
Outdated
| @@ -10,7 +10,7 @@ | |||
| <GenerateAssemblyProductAttribute>false</GenerateAssemblyProductAttribute> | |||
| <GenerateAssemblyTitleAttribute>false</GenerateAssemblyTitleAttribute> | |||
|
|
|||
| <TargetFrameworks>net45</TargetFrameworks> | |||
| <TargetFrameworks>netstandard1.3;net45</TargetFrameworks> | |||
There was a problem hiding this comment.
for the new target there should be corresponding test project. It will be basically a copy paste of net45. We copying them as Visual Studio test explorer doesn't understand yet multiuple target platfgorms in a single test project.
There was a problem hiding this comment.
Have added netcore-unit-test-projects for NLog and log4net
There was a problem hiding this comment.
Thanks for contributing!
Can you please update changelog as well with a one liner describing this change/link to GH issue.
https://github.com/Microsoft/ApplicationInsights-dotnet-logging/blob/develop/CHANGELOG.md
|
@cijothomas Updated changelog.md with a 3 liner |
|
|
||
| IDictionary<string, string> propertyBag; | ||
|
|
||
| if (trace is ExceptionTelemetry) |
There was a problem hiding this comment.
Would it help to use the ISupportProperties interface here? i.e.
propertyBag = ((ISupportProperties)trace).Properties;There was a problem hiding this comment.
I'm just moving code around (stylecop) to fix the unit tests (No PrivateObject in NetCore)
There was a problem hiding this comment.
Ah, I see. Fine to leave it as it is, then.
|
|
||
| if (!string.IsNullOrEmpty(logEvent.LoggerName)) | ||
| { | ||
| propertyBag.Add("LoggerName", logEvent.LoggerName); |
There was a problem hiding this comment.
Consider nameof(logEvent.LoggerName)
There was a problem hiding this comment.
I'm just moving code around (stylecop) to fix the unit tests (No PrivateObject in NetCore)
There was a problem hiding this comment.
Fine to leave it as it is, then.
CHANGELOG.md
Outdated
| ### Version 2.6.0-beta3 | ||
| - [NetStandard Support for NLog and log4net](https://github.com/Microsoft/ApplicationInsights-dotnet-logging/pull/167) | ||
| - [NLog and log4net can Flush](https://github.com/Microsoft/ApplicationInsights-dotnet-logging/pull/167) | ||
| - Update log4net reference to [2.0.6](https://www.nuget.org/packages/log4net/2.0.6) |
There was a problem hiding this comment.
While you're here, please could you also add a line indicating that we also added .Net Standard support for TraceListener:
- [NetStandard Support for TraceListener](https://github.com/Microsoft/ApplicationInsights-dotnet-logging/pull/166)
|
When will this be released? Thanks in advance! |
|
@MS-TimothyMothra is working on next beta as we speak. Timothy, can you confirm the estimated date of this release? |
|
Earliest will be end of next week (Apr 13th) maybe slipping to early the week of the 16th. This release is dependent on dotnet and dotnet-server. Those are in progress now and should be finished early-mid next week. (Apr 9-11). |
|
Microsoft.ApplicationInsights.NLogTarget ver. 2.6.0-beta3 has been released: https://www.nuget.org/packages/Microsoft.ApplicationInsights.NLogTarget/2.6.0-beta3 |
Updated to NLog ver. 4.5.0 RTM
Updated to log4net ver. 2.0.8 RTM
Added support for Flush()
Resolves #155 + Resolves #68 + Resolves #115