-
Notifications
You must be signed in to change notification settings - Fork 26.2k
feat(compiler-cli): ngcc - make logging more configurable #29591
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
Conversation
d520e0d
to
3259fc6
Compare
I'm going to open up the API a bit more... |
This allows CLI usage to filter excessive log messages and integrations like webpack plugins to provide their own logger. // FW-1198
3259fc6
to
92d26d3
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.
LGTM.
export const WARN = `${YELLOW}Warning:${RESET}`; | ||
export const ERROR = `${RED}Error:${RESET}`; | ||
|
||
export enum LogLevel { |
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.
Do we need a log
LogLevel?
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 don't know. We don't currently appear to :-) We can always add it later.
* The function will recurse into directories that start with `@...`, e.g. `@angular/...`. | ||
* @param sourceDirectory An absolute path to the root directory where searching begins. | ||
*/ | ||
private walkDirectoryForEntryPoints(sourceDirectory: AbsoluteFsPath): EntryPoint[] { |
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 don't see these functions using the logger at all - is the fact that they're now private class methods instead of top-level functions related to the logging change?
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.
Yes the getEntryPointsForPackage()
method calls getEntryPointInfo()
, which needs a logger.
I was really unhappy with having to pass the Logger
down to that free standing function but as much as a played around I could not get a better architecture, without a significant refactoring of this whole bit of the codebase.
) This allows CLI usage to filter excessive log messages and integrations like webpack plugins to provide their own logger. // FW-1198 PR Close angular#29591
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This allows CLI usage and integrations like webpack plugins
to filter excessive log messages.
// FW-1198