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

feat: streaming debug logfile #4062

Merged
merged 1 commit into from
Dec 2, 2021
Merged

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Nov 18, 2021

This decouples the log file writing from the terminal logging. We now
open an append only file at the start of the process and stream logs to
it. We still only display the log file message in timing mode or if
there is an error, but the file is still written regardless.

All logging now goes through proc-log and this is the first step to
removing npmlog. For now npmlog is still used for the terminal
logging but with a shim in front of it to make it easier to test and
use in conjunction with proc-log. Ref: npm/statusboard#366

This also refactors many of the tests to always use an explicit
t.testdir for their cache since the file is opened on each new Npm().
Tests are also refactored to use more of MockNpm with behavior to
add config items and load npm if necessary. A new fixture mockGlobals
was also added to make much of this more ergonomic. Ref: npm/statusboard#410

Closes npm/statusboard#411
Closes npm/statusboard#367

@lukekarrys lukekarrys changed the base branch from latest to release-next November 18, 2021 17:52
@lukekarrys lukekarrys force-pushed the lk/stream-log-file branch 16 times, most recently from 4394532 to 64e70ba Compare November 30, 2021 01:09
@lukekarrys lukekarrys marked this pull request as ready for review November 30, 2021 01:18
@lukekarrys lukekarrys requested a review from a team as a code owner November 30, 2021 01:18
@darcyclarke darcyclarke added semver:minor new backwards-compatible feature Release 8.x work is associated with a specific npm 8 release labels Nov 30, 2021
@lukekarrys lukekarrys changed the title Log file feat: streaming debug logfile Nov 30, 2021
lib/commands/exec.js Outdated Show resolved Hide resolved
lib/commands/init.js Outdated Show resolved Hide resolved
lib/npm.js Show resolved Hide resolved
lib/utils/display.js Outdated Show resolved Hide resolved
@lukekarrys lukekarrys force-pushed the lk/stream-log-file branch 4 times, most recently from a87c1c9 to 08279b9 Compare December 1, 2021 03:00
@lukekarrys lukekarrys force-pushed the lk/stream-log-file branch 14 times, most recently from eb30d29 to 3437a53 Compare December 2, 2021 00:35
This decouples the log file writing from the terminal logging. We now
open an append only file at the start of the process and stream logs to
it. We still only display the log file message in timing mode or if
there is an error, but the file is still written regardless.

All logging now goes through `proc-log` and this is the first step to
removing `npmlog`. For now `npmlog` is still used for the terminal
logging but with a shim in front of it to make it easier to test and
use in conjunction with `proc-log`. Ref: npm/statusboard#366

This also refactors many of the tests to always use an explicit
`t.testdir` for their cache since the file is opened on each `new Npm()`.
Tests are also refactored to use more of `MockNpm` with behavior to
add config items and load `npm` if necessary. A new fixture `mockGlobals`
was also added to make much of this more ergonomic. Ref: npm/statusboard#410

Closes npm/statusboard#411
Closes npm/statusboard#367

PR-URL: #4062
Credit: @lukekarrys
Close: #4062
Reviewed-by: @wraithgar
@lukekarrys lukekarrys merged commit 6734ba3 into release-next Dec 2, 2021
@npm npm deleted a comment from Branscott1780 Dec 2, 2021
@lukekarrys lukekarrys deleted the lk/stream-log-file branch December 2, 2021 15:16
@wraithgar wraithgar mentioned this pull request Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release 8.x work is associated with a specific npm 8 release semver:minor new backwards-compatible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants