console transport: send all levels to stdout not stderr by default #1332
Conversation
There was a problem hiding this comment.
@DABH code you wrote is 👍, but I think there may have been some confusion around the nature of the change itself.
The goal I meant to outline in my comment was to have all logging content be sent to stdout, not just the debug level.
I've made comments on what needs to change based on that expectation. If you can give this another pass then we'll be good to go here ^_^
lib/winston/transports/console.js
Outdated
| * @returns {mixed} - TODO: add return description. | ||
| * @param {mixed} levels - An Array of string names of levels whose output | ||
| * should go to stderr | ||
| * @param {mixed} debugStdout - Whether the 'debug' level should be sent to |
There was a problem hiding this comment.
Just remove this option since debugStdout is now the default behavior.
lib/winston/transports/console.js
Outdated
| * should go to stderr | ||
| * @param {mixed} debugStdout - Whether the 'debug' level should be sent to | ||
| * stdout (default true as of `winston@3`). | ||
| * @returns {mixed} - An Object containing the levels to send to stderr. | ||
| * @private | ||
| */ | ||
| _getStderrLevels(levels, debugStdout) { |
There was a problem hiding this comment.
This whole function can be removed since by default all levels should now go to stdout unless the user explicitly provides logging levels they would like to go to stderr.
There was a problem hiding this comment.
Gotcha. The constructor now takes stderrLevels as an optional string array as one of the potential options. Defaults to {}.
test/transports/console.test.js
Outdated
| @@ -56,7 +56,7 @@ function assertStderrLevels(transport, stderrLevels) { | |||
|
|
|||
| describe('Console transport', function () { | |||
| describe('with defaults', function () { | |||
| it('logs all levels (EXCEPT error and debug) to stdout', function () { | |||
| it('logs all levels (EXCEPT error) to stdout', function () { | |||
There was a problem hiding this comment.
It's just all levels now:
it('logs all levels to stdout', function () {
test/transports/console.test.js
Outdated
| }); | ||
|
|
||
| it("should set stderrLevels to ['error', 'debug'] by default", assertStderrLevels( | ||
| it("should set stderrLevels to ['error'] by default", assertStderrLevels( |
There was a problem hiding this comment.
it('should set stderrLevels to [] by default', assertStderrLevels(
test/transports/console.test.js
Outdated
| transports.defaults, | ||
| ['error', 'debug'] | ||
| ['error'] |
There was a problem hiding this comment.
This would just be an empty array now.
[]
test/transports/console.test.js
Outdated
| @@ -74,14 +74,14 @@ describe('Console transport', function () { | |||
| stdMocks.restore(); | |||
| var output = stdMocks.flush(); | |||
| assume(output.stderr).is.an('array'); | |||
| assume(output.stderr).length(2); | |||
| assume(output.stderr).length(1); | |||
There was a problem hiding this comment.
assume(output.stderr).length(0);
test/transports/console.test.js
Outdated
| assume(output.stdout).is.an('array'); | ||
| assume(output.stdout).length(5); | ||
| assume(output.stdout).length(6); |
There was a problem hiding this comment.
assume(output.stdout).length(7);
UPGRADE-3.0.md
Outdated
| @@ -26,6 +26,7 @@ | |||
| ### Transports | |||
| - `winston.transports.Memory` was removed. Use any Node.js `stream.Writeable` with a large `highWaterMark` instance instead. | |||
| - When writing transports use `winston-transport` instead of `winston.Transport` | |||
| - In `winston.transports.Console`, log level 'debug' ouptut is now sent to stdout by default rather than stderr (controlled by `debugStdout` option) | |||
There was a problem hiding this comment.
- In
winston.transports.Consoleoutput for all log levels is now sent to stdout by default.
debugStdoutoption has been removed.stderrLevelsnow defaults to[]
|
Thanks for the review @indexzero ! I believe these address all your comments but just let me know. Thanks! |
DABH
changed the title
…instonjs#1332) * console transport: send 'debug' level output to stdout not stderr by default * fix style * Address PR comments / everything goes to stdout by default * update doc
This addresses the common points brought up in #927 and #1024 . This change makes 'debug' level output go to stdout not stderr by default for the console transport. The behavior is still controllable by specifying the
debugStdoutoption for the transport or by passing alevelsarray, just as before. Tests updated to reflect new default and line added to upgrade guide to mention this since it's a breaking change.