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.

Sign up for GitHub

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

Jump to bottom

Added NLogProviderOptions.IncludeActivtyIdsWithBeginScope, added .NET 5 build, move tests to .NET Core LTS #456

Merged
merged 13 commits into from
Dec 29, 2020
Merged

Added NLogProviderOptions.IncludeActivtyIdsWithBeginScope, added .NET 5 build, move tests to .NET Core LTS #456

merged 13 commits into from
Dec 29, 2020

Conversation

snakefoot
Copy link
Contributor

Attempt to resolve #445

@snakefoot snakefoot added this to the 1.6.6 milestone Nov 23, 2020
@snakefoot snakefoot mentioned this pull request Nov 23, 2020
Closed
@codecov-io
Copy link

Codecov Report

Merging #456 (a2f8d2c) into master (9c3c6f0) will decrease coverage by 21.69%.
The diff coverage is 42.85%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #456       +/-   ##
===========================================
- Coverage   82.09%   60.40%   -21.70%     
===========================================
  Files          14       13        -1     
  Lines        1212     1096      -116     
  Branches      202      194        -8     
===========================================
- Hits          995      662      -333     
- Misses        144      374      +230     
+ Partials       73       60       -13
Impacted Files Coverage Δ
...Extensions.Logging/Logging/NLogBeginScopeParser.cs 64.11% <38.46%> (-2.76%) ⬇️
....Extensions.Logging/Logging/NLogProviderOptions.cs 100.00% <100.00%> (ø)
...og.Extensions.Logging/Internal/StringExtensions.cs 0.00% <0.00%> (-100.00%) ⬇️
...Logging/Config/SetupExtensionsBuilderExtensions.cs 0.00% <0.00%> (-100.00%) ⬇️
...ensions.Logging/Config/NLogLoggingConfiguration.cs 0.00% <0.00%> (-92.35%) ⬇️
...xtensions.Logging/Config/SetupBuilderExtensions.cs 0.00% <0.00%> (-83.34%) ⬇️
...tensions.Logging/Extensions/ConfigureExtensions.cs 13.11% <0.00%> (-43.25%) ⬇️
...ing/LayoutRenderers/ConfigSettingLayoutRenderer.cs 54.54% <0.00%> (-23.72%) ⬇️
...g.Extensions.Logging/Logging/NLogLoggerProvider.cs 91.22% <0.00%> (-4.02%) ⬇️
src/NLog.Extensions.Logging/Logging/NLogLogger.cs 79.78% <0.00%> (-0.73%) ⬇️
... and 3 more

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 9c3c6f0...5d5e8aa. Read the comment docs.

@304NotModified 304NotModified mentioned this pull request Dec 28, 2020
Merged
@304NotModified 304NotModified changed the title Added NLogProviderOptions.IncludeActivtyIdsWithBeginScope Added NLogProviderOptions.IncludeActivtyIdsWithBeginScope and .NET 5 build Dec 28, 2020
@304NotModified 304NotModified changed the title Added NLogProviderOptions.IncludeActivtyIdsWithBeginScope and .NET 5 build Added NLogProviderOptions.IncludeActivtyIdsWithBeginScope, added .NET 5 build, move tests to .NET Core LTS Dec 28, 2020
@304NotModified
Copy link
Member

According to this

the TFM should be "net5.0"

So changed that also.

@304NotModified
Copy link
Member

@snakefoot please let me know if you're still happy with the changes :)

snakefoot
snakefoot commented Dec 29, 2020
View reviewed changes
Copy link
Contributor Author

@snakefoot snakefoot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@304NotModified
Copy link
Member

great!

@304NotModified 304NotModified mentioned this pull request Dec 29, 2020
Closed
@repo-ranger repo-ranger bot merged commit 41b4a13 into NLog:master Dec 29, 2020
@snakefoot snakefoot mentioned this pull request Dec 29, 2020
Merged
@snakefoot
Copy link
Contributor Author

Created unit test #463 for NLogProviderOptions.IncludeActivtyIdsWithBeginScope

@304NotModified
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@304NotModified 304NotModified 304NotModified approved these changes

Assignees
No one assigned
Labels
feature size/L
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.

.NET 5.0 removes SpanId and ActivityId
3 participants
@snakefoot @codecov-io @304NotModified