-
Notifications
You must be signed in to change notification settings - Fork 327
[php-wasm-logger] Filter logs by severity in Logger and assign severity based on verbosity argument in CLIs #2436
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
--quiet
and --verbose
options in Xdebug Bridge--quiet
and --verbose
options in bridge
I will try to add tests before marking it as ready for review. |
Maybe |
@adamziel This one was a quite big refactoring. I will need a review on this. A little summary of what I had in mind when I coded these :
I therefore have several questions :
|
Things are looking good! I haven't tested it yet at this stage, only left a few comments – this one seems the most important. |
I'd say normal includes all the log levels except
Potentially? But not in this PR, it already covers a lot of ground. |
Done.
Ok! What do you think about using |
Sounds great for a follow-up PR! |
packages/playground/cli/src/blueprints-v1/blueprints-v1-handler.ts
Outdated
Show resolved
Hide resolved
I'll test this and follow up tomorrow |
I've left two notes about the code design, and my only other note would be to adjust the PR title and description to reflect the larger logging refactor. |
…Press/wordpress-playground into add-verbose-option-in-xdebug-bridge
I added the corrections you asked and modified the bridge to respect the logic from the playground :
I will now adjust the pull request title and description.
|
--quiet
and --verbose
options in bridgeThere 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.
@mho22, these changes look great to me. We've needed something like this for a while, and I think we'll be able to build on this to further improve/implement other code like the Playground CLI's --experimental-trace flag.
I left some questions and comments. Most of the comments simple "I like this" kinds of comments, but there are others too.
@mho22, if you believe this PR is complete and have no other concerns, please feel free to merge this. |
I updated the code with your suggestions. The tests passed. I guess this can be merged. |
Motivation for the change, related issues
The former intent of this pull request was to replace the console logs from the Xdebug bridge.
Console logs were replaced by Logger logs and a filter by severity was added.
The CLIs verbosity argument is now related to its CLI LogVerbosity associated with a Logger LogSeverity. This setup will filter the related logs.
Implementation details
The Playground CLI and Xdebug-bridge CLI now have a LogVerbosity variable :
The
yargs
verbosity
option is now as follows :And the Logs are filtered based on the verbosity argument :
The logger will then log the message based on if the severity level is equal or lower than the verbosity's severity level :
Testing Instructions (or ideally a Blueprint)
Normal mode
Quiet mode
Debug mode