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

.NET 4.6.1 + ASP.NET Core 2 support #223

Merged
merged 2 commits into from
Dec 1, 2017
Merged

.NET 4.6.1 + ASP.NET Core 2 support #223

merged 2 commits into from
Dec 1, 2017

Conversation

304NotModified
Copy link
Member

@304NotModified 304NotModified commented Nov 30, 2017

Fixes #208

  • introduced ASP_NET_CORE1 + ASP_NET_CORE2 constants

@snakefoot
Copy link
Contributor

snakefoot commented Nov 30, 2017

Btw. curious about ASP_NET_CORE. Since the whole project is for AspNetCore what does this define mean?

Was the intention that it should only be used for NetStandard13 + NetStandard15 + NetStandard20 ? (Right now it is also enabled also for net451 + net461)

Are there places where NetStandard2.0 should use full NetFramework logic instead of only ASP_NET_CORE ? Maybe rename ASP_NET_CORE to just NETCORE1_0 like in NLog/NLog.Extensions.Logging#171

@304NotModified
Copy link
Member Author

Btw. curious about ASP_NET_CORE. Since the whole project is for AspNetCore what does this define mean?

The code is also shared for NLog.Web (ASP.NET non-core)

Are there places where NetStandard2.0 should use full NetFramework logic instead of only ASP_NET_CORE ? Maybe rename ASP_NET_CORE to just NETCORE1_0 like in NLog/NLog.Extensions.Logging#171

Maybe, but lower prio for now I think

@304NotModified
Copy link
Member Author

update:this needs an update on NLog.Extensions.Logging -> NLog/NLog.Extensions.Logging#172

@snakefoot
Copy link
Contributor

snakefoot commented Nov 30, 2017

@304NotModified When introducing net461, then it will have the same issue as net451. See also #222

@snakefoot
Copy link
Contributor

@304NotModified net461 now has an explicit reference to NLog:

<PackageReference Include="NLog" Version="4.5.0-rc01" />

This should not be necessary. It should come from NLog.Extensions.Logging ver 1.0.0-rtm-rc3

@304NotModified
Copy link
Member Author

@snakefoot thanks!

It was a temporary trick to advoid updating nlog.extensions.logging - I was under the impression it worked, but the "build succeeded" on my machine was a false positive.

@codecov
Copy link

codecov bot commented Nov 30, 2017

Codecov Report

Merging #223 into master will decrease coverage by 1%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #223   +/-   ##
=====================================
- Coverage      59%    58%   -1%     
=====================================
  Files          29     29           
  Lines         395    395           
  Branches       92     92           
=====================================
- Hits          233    231    -2     
  Misses        126    126           
- Partials       36     38    +2
Impacted Files Coverage Δ
...youtRenderers/AspNetRequestCookieLayoutRenderer.cs 80% <0%> (-4%) ⬇️
...outRenderers/AspNetLayoutMultiValueRendererBase.cs 96% <0%> (-2%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6265c40...3cdae81. Read the comment docs.

@304NotModified
Copy link
Member Author

needs rebase on master after merge #224

@304NotModified
Copy link
Member Author

rebase done

@304NotModified
Copy link
Member Author

304NotModified commented Nov 30, 2017

Building for framework netcoreapp2.0...
  NLog.Web.AspNetCore -> C:\projects\nlogweb\NLog.Web.AspNetCore\bin\Any CPU\Debug\netstandard2.0\NLog.Web.AspNetCore.dll
  NLog.Web.AspNetCore.Tests -> C:\projects\nlogweb\NLog.Web.AspNetCore.Tests\bin\Any CPU\Debug\netcoreapp2.0\NLog.Web.AspNetCore.Tests.dll
Running .NET Core 2.0 tests for framework netcoreapp2.0...
dotnet : The specified framework version '2.0' could not be parsed
At C:\projects\nlogweb\test_aspnetcore.ps1:8 char:1
+ dotnet xunit
+ ~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (The specified f...d not be parsed:String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError
 
The specified framework 'Microsoft.NETCore.App', version '2.0' was not found.
  - Check application dependencies and target a framework version installed at:
      \
  - Alternatively, install the framework version '2.0'.
Command executed with exception: 
The specified framework 'Microsoft.NETCore.App', version '2.0' was not found.
  - Check application dependencies and target a framework version installed at:
      \
  - Alternatively, install the framework version '2.0'.

@snakefoot any idea?

in the master it also shows Building for framework netcoreapp2.0...

@304NotModified
Copy link
Member Author

ah I think it is xunit/xunit#1573

@304NotModified 304NotModified force-pushed the net461 branch 2 times, most recently from b2eff02 to 75ab6ca Compare December 1, 2017 00:08
@304NotModified 304NotModified merged commit 66f8723 into master Dec 1, 2017
@304NotModified 304NotModified deleted the net461 branch April 29, 2018 21:06
@snakefoot snakefoot added ASP.NET Core ASP.NET Core - all versions and removed ASP.NET Core 2 labels Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASP.NET Core ASP.NET Core - all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NLogBuilder not supported on .NET 4.6.1?
2 participants