Skip to content
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

fix: start consolidating color output #4781

Merged
merged 2 commits into from
May 3, 2022
Merged

fix: start consolidating color output #4781

merged 2 commits into from
May 3, 2022

Conversation

wraithgar
Copy link
Member

chalk already has a way to disable color output, so if we don't want
color we can disable it there and always use that instance of chalk.

This was only updated in the two commands that have real tests. Doing
it in the other places is going to require making their tests real so
that we don't ALSO have to rewrite their tests just to change their
internal code.

@npm-robot
Copy link
Contributor

npm-robot commented Apr 20, 2022

no statistically significant performance changes detected

timing results
app-large clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 46.431 ±0.92 28.303 ±0.31 15.902 ±0.03 18.225 ±0.59 2.551 ±0.02 2.527 ±0.01 1.998 ±0.01 10.304 ±0.05 1.993 ±0.00 2.921 ±0.02
#4781 47.368 ±1.05 28.469 ±0.29 15.716 ±0.24 18.529 ±0.99 2.587 ±0.01 2.525 ±0.00 1.975 ±0.02 10.331 ±0.08 1.983 ±0.01 3.204 ±0.35
app-medium clean lock-only cache-only cache-only
peer-deps
modules-only no-lock no-cache no-modules no-clean no-clean
audit
npm@8 33.889 ±0.13 21.789 ±0.12 11.518 ±0.05 12.419 ±0.56 2.321 ±0.01 2.328 ±0.00 2.044 ±0.00 7.552 ±0.10 1.900 ±0.00 2.615 ±0.02
#4781 35.114 ±0.67 21.901 ±0.17 11.621 ±0.05 12.525 ±0.26 2.296 ±0.01 2.292 ±0.02 2.042 ±0.03 7.548 ±0.14 1.886 ±0.01 2.632 ±0.02

chalk already has a way to disable color output, so if we don't want
color we can disable it there and always use that instance of chalk.

This was only updated in the two commands that have real tests.  Doing
it in the other places is going to require making their tests real so
that we don't ALSO have to rewrite their tests just to change their
internal code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants