New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(core): add option to disable logs on LazyModuleLoader#load
#9836
Conversation
Pull Request Test Coverage Report for Build f23ec8df-fadf-44d6-91ab-64f8a00faede
💛 - Coveralls |
bca47f9
to
c04a7f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it is much better, so the principle of "Single Responsibility" is respected, but now I have another doubt that it "torments" me it is better to leave it inside the injector folder or move it to where the Logger and the LoggerService are because the SilentLogger class extends Logger? 😄 maybe it is better to move it inside the common folder? 🤔
import { Logger } from '@nestjs/common'; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-empty-function | ||
const noop = () => {}; | ||
export class SilentLogger extends Logger { | ||
log = noop; | ||
error = noop; | ||
warn = noop; | ||
debug = noop; | ||
verbose = noop; | ||
setLogLevels = noop; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it is much better, so the principle of "Single Responsibility" is respected, but now I have another doubt that it "torments" me it is better to leave it inside the injector folder or move it to where the Logger and the LoggerService are because the SilentLogger class extends Logger? 😄 maybe it is better to move it inside the common folder? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if we should move that to common dir as that class isn't being used by any other module other than injector. But yeah..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact, we could move it to the common folder, but perhaps it is better that we wait to see what Kamil thinks about this move. 😄👍
lgtm |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: closes #9107
What is the new behavior?
I choose the name
logger
so we could enhance this feature later on without introducing breaking changes on that signatureDoes this PR introduce a breaking change?
Other information