-
Notifications
You must be signed in to change notification settings - Fork 15
feat(ci): improve code-pushup command logs #1152
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
|
View your CI Pipeline Execution ↗ for commit ff64ed9
☁️ Nx Cloud last updated this comment at |
@code-pushup/ci
@code-pushup/cli
@code-pushup/core
@code-pushup/create-cli
@code-pushup/models
@code-pushup/nx-plugin
@code-pushup/axe-plugin
@code-pushup/coverage-plugin
@code-pushup/js-packages-plugin
@code-pushup/eslint-plugin
@code-pushup/jsdocs-plugin
@code-pushup/lighthouse-plugin
@code-pushup/typescript-plugin
@code-pushup/utils
@code-pushup/models-transformers
commit: |
22eca8b to
813c37a
Compare
Code PushUp🤨 Code PushUp report has both improvements and regressions – compared current commit 4ea2a25 with previous commit eb876a8. 🕵️ See full comparison in Code PushUp portal 🔍 🏷️ Categories👍 2 groups improved, 👎 1 group regressed, 👍 5 audits improved, 👎 5 audits regressed, 13 audits changed without impacting score🗃️ Groups
20 other groups are unchanged. 🛡️ Audits
655 other audits are unchanged. |
0232e63 to
ff64ed9
Compare
hanna-skryl
left a comment
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.
Nice to see the log group rendering fixed 👍 I wonder if there is a specific reason some JSON logs have remained non-collapsible.
I'm guessing you mean the npm commands with large JSON output? That's the raw process output, which is meant to be printed underneath the spinner with indentation, as you see. But it shouldn't be logged by default - only the command name, status, and duration are always logged. However, Michael hard-coded |
After reviewing how our CI logs look with the new Logger, I decided some further improvements are needed to make them readable.
The new way of logging executed processes nicely formats external tools, but when the CI package logs
code-pushupcommands this way, it introduces too much nesting. This also breaks log group rendering.Furthermore, I realized a single
verboseflag for everything isn't the best in this case. The CI doesn't log almost anything without enablingCP_VERBOSE, because most of the interesting output is from thecode-pushupcommands (e.g., report summary). However, verbose shouldn't be needed by default, as it introduces too much noise (e.g., all passed audits).My solution is to disable the default process logging in the CI package, and use some new low-level utilities to print
code-pushupoutput as soon as it's received (don't have to wait for everything to complete before seeing progress), with spaces via line breaks instead of indentation and gray color. By default, the output is printed, regardless ofCP_VERBOSE. To silence thecode-pushupcommand output specifically, I've introduced an optionalsilentflag.For all other
executeProcesscalls, the logs look the same as before.