Logger refactor: log message polishing - #12
Conversation
| import { ILogger } from '../../logger/types'; | ||
| import sanitizeValue from './sanitize'; | ||
| import { DEBUG_24, WARN_1, WARN_0 } from '../../logger/constants'; | ||
| import { DEBUG_24, WARN_ENGINE_NO_ATTRIBUTES, WARN_ENGINE_INVALID_VALUE } from '../../logger/constants'; |
There was a problem hiding this comment.
FTR: Let's use this format instead of a numeric one. It's clearer!
There was a problem hiding this comment.
done in next commit
| import { DEBUG_28, DEBUG_29, DEBUG_30, ERROR_1 } from '../logger/constants'; | ||
| import { CLEANUP_LB, DEBUG_26, DEBUG_27 } from '../logger/constants'; | ||
|
|
||
| const LOG_EXTRA = 'for SIGTERM signal.'; |
There was a problem hiding this comment.
When polishing, try to find a more descriptive name.
|
|
||
| stop() { | ||
| this.settings.log.debug(DEBUG_29); | ||
| this.settings.log.debug(DEBUG_27, [LOG_EXTRA]); |
There was a problem hiding this comment.
In this file, the logs have a different format, but all of them have something appended at the end. Not necessary now, but eventually we should try and be consistent if possible.
| @@ -1,151 +1,140 @@ | |||
| // commons | |||
There was a problem hiding this comment.
Re-order the messages in the file pls.
There was a problem hiding this comment.
will be done in a coming commit, when renaming constants
| const formattedText = this._generateLogMessage(level, text); | ||
| if (typeof msg === 'number') { | ||
| const format = this.codes.get(msg); | ||
| if (format) msg = sprintf(format, args); |
There was a problem hiding this comment.
maybe this should run for string messages too.
There was a problem hiding this comment.
yes! good idea. It was added in next PR
|
|
||
| export const codesError: [number, string][] = [ | ||
| // evaluator | ||
| [ERROR_0, ENGINE_COMBINER_LB + 'Invalid Split provided, no valid conditions found'], |
There was a problem hiding this comment.
let's rephrase to: "Invalid Split, no valid rules found"
EmilianoSanchez
changed the title
2 participants
Javascript commons library
What did you accomplish?
How do we test the changes introduced in this PR?
Extra Notes