Discord adapter#27
Conversation
| type Config = LoggerAdapterConfig & { | ||
| webhookId: string, | ||
| token: string, | ||
| regex: RegExp, |
| [SupportedLogLevels.Warn]: 3, | ||
| [SupportedLogLevels.Info]: 4, | ||
| [SupportedLogLevels.Debug]: 5, | ||
| [SupportedLogLevels.Debug]: 6, |
| 'content': message, | ||
| }); | ||
| } catch (err) { | ||
| console.log('Something when wrong', err); |
There was a problem hiding this comment.
Why don't let the error propagate to the appropriate error handler instead?
| } | ||
|
|
||
| public log(message: LogMessage): void { | ||
| const formattedMessageArray = this.formatMessage(message); |
There was a problem hiding this comment.
perhaps format the message only if any of the args match regex?
| public log(message: LogMessage): void { | ||
| const formattedMessageArray = this.formatMessage(message); | ||
|
|
||
| if (formattedMessageArray.find((fmessage) => this.regex.test(fmessage))) { |
There was a problem hiding this comment.
Why is this necessary in order for the message to get logged? Looking at the other adapters, these are supposed to provide a general-purpose implementation which this functionality doesn't quite fit in. A better solution to this would be to create a decorator with the same interface as the rest of the adapters, but which will only log messages if they're matching certain criteria i.e. log level, arguments etc. This way we could implement application-specific logic, without polluting the logging library in general.
| 'content': message, | ||
| }); | ||
| } catch (err) { | ||
| console.log('Something when wrong', err); |
There was a problem hiding this comment.
Why don't let the error propagate to the appropriate error handler instead?
| } | ||
| } | ||
|
|
||
| private formatMessage(message: LogMessage) { |
There was a problem hiding this comment.
How does a message formatted using this method look in Discord? Isn't there a better way to format the messages using its own formatting capabilities? For example:
Or rather have the format configurable.
| { | ||
| "name": "@luckbox/logger-factory", | ||
| "version": "4.0.0", | ||
| "version": "5.0.0", |
There was a problem hiding this comment.
The proposed changes don't mandate a major version bump.
| }, | ||
| "engines": { | ||
| "node": ">=12.14" | ||
| "node": ">=16" |
There was a problem hiding this comment.
I don't think we're using anything that isn't present in Node 12, so this constraint seems rather unnecessary.
3 participants
No description provided.