-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add default logmanager #112
base: development
Are you sure you want to change the base?
Conversation
…added some extra logging.
…o using the LogManager.
…n to one level higher so we can log it more easily.
Looks good, but one small question. Will it ever work? You are using |
return; | ||
} | ||
|
||
if (_log.IsDebugEnabled()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this check be moved to the Debug
method itself. Pretty error prone if you have to check this yourself everytime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The debug method itself should have it as well, the only thing the _log.IsDebugEnabled() checks will make sure that the string won't be computed. Since these are in the critical path for request handling I didn't want the strings to be computed.
Is this something we should still look at? |
I'll take a look at this at some point, but probably not to soon. Might have some time this weekend. |
As discussed in #111, this will add a default log manager which will log to the debug console to make it so that we can detect errors in setup and operation easier.