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

Write internallog warning for unrecognized properties used in config on layout renderers #3896

Merged
merged 1 commit into from Apr 19, 2020

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Apr 14, 2020

LayoutParser - InternalLogger Warnings when skipping unrecognized properties for LayoutRenderers

@snakefoot snakefoot added enhancement Improvement on existing feature nlog-configuration labels Apr 14, 2020
@snakefoot snakefoot added this to the 4.7.1 milestone Apr 14, 2020
@snakefoot snakefoot force-pushed the LayoutParserSkipWithWarning branch 7 times, most recently from 563d86f to 7f35bd3 Compare April 15, 2020 15:09
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 15, 2020
@snakefoot snakefoot force-pushed the LayoutParserSkipWithWarning branch 2 times, most recently from 6ed2957 to f260174 Compare April 15, 2020 19:13
@304NotModified
Copy link
Member

Hi @snakefoot

Thanks for the PR. Unfortunately I don't fully understand what we here trying to achieve. Does this PR add a new warning to the internal log when something is skipped? What is an example of "something" here?

PS: you could create drafts now. Please do, because I get a notification for every push now (have converted to draft now). If you think it's ready for review, please undraft. Thanks!

@304NotModified 304NotModified marked this pull request as draft April 15, 2020 19:18
@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 15, 2020 via email

@snakefoot snakefoot marked this pull request as ready for review April 15, 2020 20:10
@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 15, 2020

Oops. Had to fix some unnecessary Type-Name-Lookup when doing debug-logging in ApplyWrappers

@304NotModified 304NotModified changed the title LayoutParser - InternalLogger Warnings when skipping unrecognized properties If unrecognized property is skipped when reading NLog's config, write a warning to the internal log Apr 18, 2020
@304NotModified
Copy link
Member

I would like to have a warning when that happens. The PR is now done.

Cool! I renamed the PR, maybe a bit long. Feel free to rename it (but I prefer to keep it "functional")

@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 18, 2020

Fine with me. Though the warning is only for LayoutRenderers

@sonarcloud
Copy link

sonarcloud bot commented Apr 18, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 4 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 18, 2020

Looked at the code for assigning default-parameter-value. I guess if one is doing this:

${exception }

Then one will not get any output, as it would double space ' ' to the default-parameter Format.

Should add validation that one is doing this (Notice the :)

${exception: }

And if by accident adding a space for a layout-renderer without a registered, default-parameter, then it would generate confusing error message about type not having default-parameter-property.

@304NotModified 304NotModified changed the title If unrecognized property is skipped when reading NLog's config, write a warning to the internal log Write internallog warning for unrecognized properties used in config on layout renderers Apr 19, 2020
@304NotModified 304NotModified merged commit fc5f5ca into NLog:master Apr 19, 2020
@304NotModified
Copy link
Member

Thanks!

@snakefoot snakefoot deleted the LayoutParserSkipWithWarning branch July 30, 2022 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement on existing feature nlog-configuration size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants