-
Notifications
You must be signed in to change notification settings - Fork 774
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
log4js.configure({}) resets all loggers' levels to TRACE #70
log4js.configure({}) resets all loggers' levels to TRACE #70
Conversation
… to TRACE, even if they were previously set to something else
Would you mind adding some tests to cover this, please? |
…ls' property is passed in the configuration
Done |
log4js.configure({}) resets all loggers' levels to TRACE
Thank you very much. |
Tests fail for me in node 0.6.15, example below: Setting up loggers with initial levels, then setting a configuration which has null levels, and checking the logger whose level was set to Warn with isLevelEnabled(Debug)
|
Never mind - I fixed it :) |
What was the problem? I had some similar issues originally, which disappeared after I hosed everything and went to a clean node v0.6.7, vows 0.6.2, and HEAD of log4js-node/master |
Here's the commit: 613a077 |
OK, since that's the only place setLevel() is called with no params, the previous change to line 204 of log4js.js probably isn't needed now. Still, if it works it works ;) |
It's still failing on travis, but I think that's an npm problem for me to investigate. http://travis-ci.org/#!/nomiddlename/log4js-node |
This is seems inconsistent, given that log4js.configure({levels:{}}) preserves any previously set levels.
https://gist.github.com/2577583
This fixes that, by ensuring the setLevel(undefined || null) does not over-write a previously set level with TRACE.
Since the only time setLevel() is called within log4js.js with no parameters is when configure is called without the configuration object containing a 'levels' property, this seems like a reasonable and simple fix.