Skip to content

fix(log): pass in logger to external modules - #4359

Merged
wraithgar merged 1 commit into
release-nextfrom
gar/log-to-commands
Feb 3, 2022
Merged

fix(log): pass in logger to external modules#4359
wraithgar merged 1 commit into
release-nextfrom
gar/log-to-commands

Conversation

@wraithgar

Copy link
Copy Markdown
Contributor

Most of these module use npm-registry-fetch under the hood, which will
log things like the npm-notice header if seen. Currently we aren't
passing in a logger to them, which means that log message is never seen,
among any other logged messages those modules may need to make.

I added tests where I could. Some tests were in a state where the entire
libnpm* module was an empty mocked function, so asserting that it got
passed a log attribute was onerous.

@wraithgar
wraithgar requested a review from a team as a code ownerFebruary 1, 2022 22:46
@npm-robot

npm-robot commented Feb 1, 2022

Copy link
Copy Markdown
Contributor

found 20 benchmarks with statistically significant performance improvements

  • app-large: clean, lock-only, cache-only, cache-only:peer-deps, modules-only, no-lock, no-cache, no-modules, no-clean, no-clean:audit
  • app-medium: clean, lock-only, cache-only, cache-only:peer-deps, modules-only, no-lock, no-cache, no-modules, no-clean, no-clean:audit
timing results
app-largecleanlock-onlycache-onlycache-only
peer-deps
modules-onlyno-lockno-cacheno-modulesno-cleanno-clean
audit
npm@856.306 ±0.5336.000 ±0.1822.488 ±0.0925.277 ±0.563.635 ±0.043.659 ±0.142.914 ±0.0514.984 ±0.002.881 ±0.034.079 ±0.09
#43590.479 ±0.000.522 ±0.030.482 ±0.000.481 ±0.010.485 ±0.010.486 ±0.030.490 ±0.000.517 ±0.020.492 ±0.000.486 ±0.01
app-mediumcleanlock-onlycache-onlycache-only
peer-deps
modules-onlyno-lockno-cacheno-modulesno-cleanno-clean
audit
npm@843.522 ±2.1227.296 ±0.0016.257 ±0.0817.641 ±0.023.356 ±0.013.297 ±0.082.947 ±0.0811.051 ±0.302.763 ±0.033.631 ±0.10
#43590.483 ±0.010.470 ±0.010.462 ±0.000.504 ±0.000.504 ±0.030.505 ±0.010.469 ±0.000.491 ±0.010.480 ±0.010.495 ±0.01

Most of these module use npm-registry-fetch under the hood, which will
log things like the `npm-notice` header if seen. Currently we aren't
passing in a logger to them, which means that log message is never seen,
among any other logged messages those modules may need to make.
I added tests where I could. Some tests were in a state where the entire
libnpm* module was an empty mocked function, so asserting that it got
passed a `log` attribute was onerous.
@ruyadornoruyadorno added Release 8.x work is associated with a specific npm 8 release release: next These items should be addressed in the next release semver:patch semver patch level for changes labels Feb 3, 2022
@wraithgar
wraithgar merged commit 1b93385 into release-nextFeb 3, 2022
@wraithgar
wraithgar deleted the gar/log-to-commands branch February 3, 2022 14:43
@ruyadornoruyadorno mentioned this pull request Feb 3, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: nextThese items should be addressed in the next releaseRelease 8.xwork is associated with a specific npm 8 releasesemver:patchsemver patch level for changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@wraithgar@npm-robot@ruyadorno