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

NLog - NETSTANDARD1_5 #2341

Merged
merged 2 commits into from
Oct 12, 2017
Merged

NLog - NETSTANDARD1_5 #2341

merged 2 commits into from
Oct 12, 2017

Conversation

snakefoot
Copy link
Contributor

Just checking if it can make a proper build

@snakefoot snakefoot force-pushed the NetStandard15 branch 6 times, most recently from a2a1847 to 306d7d4 Compare October 12, 2017 05:15
@codecov
Copy link

codecov bot commented Oct 12, 2017

Codecov Report

Merging #2341 into master will increase coverage by <1%.
The diff coverage is 89%.

@@           Coverage Diff           @@
##           master   #2341    +/-   ##
=======================================
+ Coverage      82%     82%   +<1%     
=======================================
  Files         317     318     +1     
  Lines       22720   22771    +51     
  Branches     2782    2782            
=======================================
+ Hits        18620   18666    +46     
- Misses       3396    3402     +6     
+ Partials      704     703     -1

@304NotModified
Copy link
Member

304NotModified commented Oct 12, 2017

looks good! (only checked the build result ;))

@304NotModified
Copy link
Member

Think we should merge it after resolving the conflict?

I'm doubting about the unit tests. (enabling for netstandard1.5)

@snakefoot
Copy link
Contributor Author

snakefoot commented Oct 12, 2017

I'm doubting about the unit tests. (enabling for netstandard1.5)

Allmost all tests are working after removing all those for features not available (ex. NLogTraceListener). Except the tests that checks current-class-logger for inner-classes where NetStandard1_5 uses "." as last delimiter (instead of "+")

But it will require some effort to force the NetCoreApp2 to load the NetStandard1_5-dll.

@304NotModified
Copy link
Member

But it will require some effort to force the NetCoreApp2 to load the NetStandard1_5-dll.

run it as NetCoreApp1?

@304NotModified
Copy link
Member

304NotModified commented Oct 12, 2017

@snakefoot
Copy link
Contributor Author

run it as NetCoreApp1?

Well then I have to fix the UnitTest-project to work under NetStandard1_5

@snakefoot
Copy link
Contributor Author

@304NotModified after resolving the conflict?

Conflicts resolved.

@304NotModified
Copy link
Member

should the unit tests run in .net standard 1.5, as you have changed the testbase?

@snakefoot
Copy link
Contributor Author

snakefoot commented Oct 12, 2017

should the unit tests run in .net standard 1.5, as you have changed the testbase?

I have just made some changes, so it was easier to make local hack of adding NETSTANDARD1_5 to unit-test-project for NetCoreApp2 and removing NetStandard2.0 from Nlog.dll

Not in any condition to perform the NetStandard1.5 testing in current state on the build-server. Would also have to fix the 10 tests that detects behavior change in StrackTrace-logic.

@304NotModified 304NotModified added this to the 4.5 beta 7 milestone Oct 12, 2017
@304NotModified
Copy link
Member

OK thanks! Maybe later. I think we could run the tests on Travis in the future

@snakefoot
Copy link
Contributor Author

@304NotModified Guess you could create an issue about the missing testing of the NetStandard1.5 assembly (Like #1186 and #1184)

@snakefoot snakefoot mentioned this pull request Nov 23, 2017
@snakefoot snakefoot removed this from the 4.5 beta 7 milestone Aug 22, 2023
@snakefoot snakefoot added this to the 4.5 milestone Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants