Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add logs-dir option #4594

Merged
merged 3 commits into from
Mar 24, 2022
Merged

feat: add logs-dir option #4594

merged 3 commits into from
Mar 24, 2022

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Mar 22, 2022

Requires: npm/npm-registry-fetch#107

This also allows logs-max to be set to 0 to disable log file writing.

Closes #4466
Closes #4206

@npm-robot
Copy link
Contributor

npm-robot commented Mar 22, 2022

found 2 benchmarks with statistically significant performance regressions

  • app-large: clean, no-clean:audit
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 53.528 ±1.97 34.176 ±0.79 30.485 ±13.13 23.949 ±1.20 3.373 ±0.00 3.530 ±0.06 2.853 ±0.06 14.032 ±0.02 2.836 ±0.03 3.862 ±0.10
#4594 59.775 ±1.57 34.754 ±0.18 21.901 ±0.96 24.484 ±1.22 3.643 ±0.11 3.626 ±0.17 2.788 ±0.04 14.509 ±0.00 2.915 ±0.01 4.297 ±0.16
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 43.734 ±2.75 26.167 ±0.19 15.547 ±0.26 16.923 ±0.55 3.173 ±0.02 3.316 ±0.07 2.988 ±0.00 10.622 ±0.05 2.695 ±0.04 3.689 ±0.04
#4594 46.480 ±0.46 26.839 ±0.20 16.371 ±0.11 16.966 ±0.17 3.249 ±0.17 3.136 ±0.03 2.975 ±0.22 10.609 ±0.03 2.686 ±0.01 3.614 ±0.10

@lukekarrys lukekarrys force-pushed the lk/redact-log branch 8 times, most recently from bc5d02b to 4a6a890 Compare March 22, 2022 05:38
@lukekarrys lukekarrys marked this pull request as ready for review March 22, 2022 05:38
@lukekarrys lukekarrys requested a review from a team as a code owner March 22, 2022 05:38
lib/npm.js Show resolved Hide resolved
lib/npm.js Show resolved Hide resolved
Copy link
Member

@wraithgar wraithgar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of good cleanup in this feat.

@lukekarrys lukekarrys force-pushed the lk/redact-log branch 2 times, most recently from 6071834 to 4f98842 Compare March 23, 2022 00:58
@lukekarrys
Copy link
Contributor Author

Just a note that this PR should probably be rebased, as I added the deps as separate commits.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
4 participants