Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Add logger name to non-pretty loggers #15

Merged
merged 9 commits into from
May 28, 2019
Merged

Conversation

vlasy
Copy link
Contributor

@vlasy vlasy commented May 22, 2019

And fix logger name for pretty loggers

@vlasy vlasy force-pushed the change/logger-name-nonpretty branch from cdb52c7 to 1bc3d75 Compare May 22, 2019 14:31
@vlasy vlasy requested a review from smoliji May 23, 2019 06:16
Copy link
Contributor

@smoliji smoliji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now consfused with the differencies between moduleName and the new loggerName.

The purpose of the moduleName should've been to create a named module, so I could disable logging for a certain part (module) of the application. Whereas the loggerName propagates the given name to each of the logs.

If this is the case, I suggest to merge those two - use a name attribute (e.g. moduleName) and propagate this name to every log.

src/tests/index.test.ts Outdated Show resolved Hide resolved
@vlasy
Copy link
Contributor Author

vlasy commented May 23, 2019

I am now consfused with the differencies between moduleName and the new loggerName.

The situation before was following: if you passed a string to loggerFactory, named logger was created. Therefore its name was shown in all pretty logs and probably was present in non-pretty logs as name property. However, if you logged any object with name key, it collided with this logger name.

After this change: If you pass a string to loggerFactory, named logger will be created. But it will no longer collide with objects with field name + it will be also visible as part of the message in non-pretty logs.

But you are right - I could rename moduleName to loggerName so this will be more explicit.

The purpose of the moduleName should've been to create a named module, so I could disable logging for a certain part (module) of the application. Whereas the loggerName propagates the given name to each of the logs.

I think this PR changes nothing about disabling loggers.

@smoliji smoliji self-requested a review May 27, 2019 14:17
@smolijar smolijar merged commit 86a621b into master May 28, 2019
@vlasy vlasy deleted the change/logger-name-nonpretty branch June 6, 2019 06:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants