Fixes #1105 - adds static Log.None property with appropriate tests. … #1108
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ests. Allowed for slight simplification of _logger initialization and CloseAndFlush() implementation
|
Hmmm - AppVeyor failed in an area that I don't think I affected. |
|
Thanks, Ian! Since this is more of a plumbing feature, than something that will be used regularly by callers of |
nblumhardt
reviewed
Feb 18, 2018
| @@ -1201,5 +1209,16 @@ public static bool BindProperty(string propertyName, object value, bool destruct | |||
| { | |||
| return Logger.BindProperty(propertyName, value, destructureObjects, out property); | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// An <see cref="ILogger"/> instance that silently ignores all log messages | |||
There was a problem hiding this comment.
Nitpick, could use a . here.
Perhaps we could try "that efficiently ignores all method calls"?
|
Not sure why I ended up on Log.None... Logger.None makes more sense. Shall amend this evening. Thanks |
2 tasks
|
Closing in favour of PR #1110 instead |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What issue does this PR address?
Adds a static
Log.Noneproperty as per the discussion in #1105Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information:
Basic change to stash a
SilentLoggerinstance in a private static variable on theLogclass. That instance is used as the initial value of theLoggerproperty. The same instance is then exposed via theNoneproperty.CloseAndFlush()was modified to reset back to this instance rather than creating yet anotherSilentLoggerinstance.Comments were updated where appropriate and code style was preserved.
I've added tests to ensure that the
Noneproperty is initialised properly. I've also ensured via a test that initialisation of theLoggerproperty doesn't affect theNoneproperty.Finally I've ensured that
CloseAndFlushdoesn't corrupt the logger stored and accessible via theNoneproperty by checking that it remains aSilentLoggerinstance and that we can invoke it without raising an exception.I think there are lines of words here than lines of code changed in the PR. All tests (on my machine) seem to pass in the various framework versions.
Thanks 💯