fluent / NLog.Targets.Fluentd

Apache License 2.0
44 stars 31 forks source link

Upgraded to VS2017 and added NetStandard 2.0 #8

Closed snakefoot closed 6 years ago

snakefoot commented 6 years ago

Also made some minor changes:

repeatedly commented 6 years ago

@moriyoshi Do you have a time for this patch?

moriyoshi commented 6 years ago

I'm taking a look into this.

snakefoot commented 6 years ago

@moriyoshi Have implemented the suggested changes.

moriyoshi commented 6 years ago

I'd like to keep this compatible with the older versions of NLog whose current minimum requirement of .NET version is 3.5, while .NET Standard 2.0 apparently requires 4.5 or above. Do you have any idea on this? Not every user can lift up their NLog requirements.

snakefoot commented 6 years ago

Your current project only supports net45, so not sure why you mention net35.

I have configured the new Nlog.Targets.Fluentd.csproj, so Net45 uses Nlog 2.1.0, and NetStandard20 uses Nlog4.5

One just have to dotnet pack and will generate a nice nuget package

moriyoshi commented 6 years ago

Your current project only supports net45, so not sure why you mention net35.

Yep, but there wasn't much intention. I'd thought the code base could be compiled with .NET 3.5 without much work at the time I published this project.

I have configured the new Nlog.Targets.Fluentd.csproj, so Net45 uses Nlog 2.1.0, and NetStandard20 uses Nlog4.5

That sounds OK. The problem I was seeing is that older NLog (<4.5) don't have structural logging capability, nor the accompanying API so it appears the new code base cannot be compiled with the older versions.

snakefoot commented 6 years ago

The structural logging is just using LogEventInfo.Properties that also exist in Nlog ver. 2.1. The Nlog.Targets.Fluentd still supports Nlog 2.1 for net45. It is only Netstandard that needs the new Nlog 4.5

I can create a new PR that adds support for net35 and net40. When this has been merged

snakefoot commented 6 years ago

@moriyoshi Have added net35 and net40 as build targets, so the nuget-package will also support those platforms (Also with NLog 2.1 as minimum version).

Because you want the now obsolete net35, then you cannot use dotnet pack. Instead you have to do it like this (From a developer command-prompt):

msbuild /t:Restore,Pack .\src\NLog.Targets.Fluentd /p:Configuration=Release /p:IncludeSymbols=true /verbosity:minimal

Or just right-click the project in Visual Studio 2017 and choose "Pack".

snakefoot commented 6 years ago

@moriyoshi Something that I'm missing, before this can be merged?

moriyoshi commented 6 years ago

Looks great! Apologies for leaving this for a long time. I think I can merge this then.