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

NLogProviderOptions - Added LoggingConfigurationSectionName #501

Merged
merged 4 commits into from
May 1, 2021

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Apr 6, 2021

Enables automatic loading of NLog-LoggingConfiguration from appsettings.json when the following conditions are true:

  • No configuration has been loaded already
  • NLogProviderOptions.LoggingConfigurationSectionName has been assigned. Ex. to "NLog"
  • The available host-configuration has the specified section, and it contains config-items.

With NLog 5.0 then the default-value can be changed to "NLog" so it will always automatically load NLog LoggingConfiguration from appsettings.json (if no other configuration has been loaded already)

@snakefoot snakefoot added this to the 1.7.3 milestone Apr 6, 2021
Copy link
Member

@304NotModified 304NotModified 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! Important is the naming, other comment is minor

@snakefoot snakefoot changed the title NLogProviderOptions - Added LoadConfigurationFromSection NLogProviderOptions - Added LoggingConfigurationSectionName Apr 28, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #501 (167b5e2) into master (084cd50) will increase coverage by 0.17%.
The diff coverage is 78.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #501      +/-   ##
==========================================
+ Coverage   80.96%   81.14%   +0.17%     
==========================================
  Files          16       16              
  Lines        1345     1379      +34     
  Branches      234      235       +1     
==========================================
+ Hits         1089     1119      +30     
+ Misses        172      171       -1     
- Partials       84       89       +5     
Impacted Files Coverage Δ
...tensions.Logging/Extensions/ConfigureExtensions.cs 55.08% <33.33%> (+1.57%) ⬆️
...tensions.Hosting/Extensions/ConfigureExtensions.cs 85.29% <83.33%> (+1.96%) ⬆️
...ns.Logging/Internal/RegisterNLogLoggingProvider.cs 96.66% <93.33%> (-3.34%) ⬇️
....Extensions.Logging/Logging/NLogProviderOptions.cs 100.00% <100.00%> (ø)
...ensions.Logging/Config/NLogLoggingConfiguration.cs 90.94% <0.00%> (-1.40%) ⬇️
...ing/LayoutRenderers/ConfigSettingLayoutRenderer.cs 82.60% <0.00%> (+4.34%) ⬆️

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 084cd50...167b5e2. Read the comment docs.

Copy link
Member

@304NotModified 304NotModified left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@snakefoot
Copy link
Contributor Author

Thank you for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants