refactor: promote logger to its own package #493
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
grypez
force-pushed
the
grypez/add-logger-package
branch
5 times, most recently
from
0515527 to
2f60400
Compare
grypez
commented
Apr 22, 2025
grypez
changed the title
grypez
force-pushed
the
grypez/add-logger-package
branch
from
2f60400 to
2a76d18
Compare
rekmarks
reviewed
Apr 23, 2025
Member
rekmarks
left a comment
rekmarks
left a comment
There was a problem hiding this comment.
Looks good! Some questions / suggestions.
Comment on lines
+13
to
+34
| /** | ||
| * Parses the options for the logger. | ||
| * | ||
| * @param options - The options for the logger. | ||
| * @returns The parsed options. | ||
| */ | ||
| export const parseOptions = ( | ||
| options: LoggerOptions | string | undefined, | ||
| ): LoggerOptions => { | ||
| // The default case catches whatever is not explicitly handled below. | ||
| // eslint-disable-next-line @typescript-eslint/switch-exhaustiveness-check | ||
| switch (typeof options) { | ||
| case 'object': | ||
| return options; | ||
| case 'string': | ||
| return { tags: [options], transports: [consoleTransport] }; | ||
| case 'undefined': | ||
| return { transports: [consoleTransport] }; | ||
| default: | ||
| throw new Error('Invalid logger options'); | ||
| } | ||
| }; |
Member
There was a problem hiding this comment.
These are a lot of options. We could just use an options bag instead. Does not need to be actioned in this PR.
grypez
force-pushed
the
grypez/add-logger-package
branch
from
3f43248 to
8969312
Compare
rekmarks
previously approved these changes
Apr 23, 2025
Member
rekmarks
left a comment
rekmarks
left a comment
There was a problem hiding this comment.
Some conflicts snuck in but otherwise LGTM!
grypez
force-pushed
the
grypez/add-logger-package
branch
from
a5c5845 to
076e45c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refs: #217
@ocap/loggerpackageLoggerclass from@ocap/utilsto@ocap/loggermakeLoggercalls withnew Loggerin refactored packagesmakeLoggerfactory from the monorepo