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

Use proc-log for stdout and stderr #7373

Merged
merged 13 commits into from
Apr 16, 2024
Merged

Use proc-log for stdout and stderr #7373

merged 13 commits into from
Apr 16, 2024

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented Apr 12, 2024

  • deps: @npmcli/git@5.0.6
  • deps: @npmcli/package-json@5.0.3
  • deps: npm-package-arg@11.0.2
  • deps: npm-profile@9.0.1
  • deps: npm-registry-fetch@16.2.1
  • deps: pacote@17.0.7
  • deps: proc-log@4.0.0
  • deps: @npmcli/run-script@8.0.0
  • feat(libnpmversion)!: remove silent option
  • feat(libnpmpack)!: remove silent option
  • feat(libnpmexec)!: no longer accept output function
  • feat: do all ouput over proc-log events

TODO

@lukekarrys lukekarrys changed the title lk/proc log deps Update proc-log and @npmcli/run-script deps Apr 12, 2024
@lukekarrys lukekarrys force-pushed the lk/proc-log-deps branch 9 times, most recently from 09526ae to 0b77f20 Compare April 13, 2024 01:44
@lukekarrys lukekarrys changed the title Update proc-log and @npmcli/run-script deps Use proc-log out stdout and stderr Apr 13, 2024
@lukekarrys lukekarrys changed the title Use proc-log out stdout and stderr Use proc-log for stdout and stderr Apr 14, 2024
@lukekarrys lukekarrys marked this pull request as ready for review April 15, 2024 22:34
@lukekarrys lukekarrys requested a review from a team as a code owner April 15, 2024 22:34
@lukekarrys lukekarrys force-pushed the lk/proc-log-deps branch 2 times, most recently from e612ae9 to d9985cc Compare April 15, 2024 23:43
@lukekarrys lukekarrys mentioned this pull request Apr 16, 2024
1 task
lib/utils/display.js Outdated Show resolved Hide resolved
lib/utils/display.js Outdated Show resolved Hide resolved
lib/utils/display.js Outdated Show resolved Hide resolved
@wraithgar
Copy link
Member

wraithgar commented Apr 16, 2024

TODO: make sure we understand why signal-exit is not in node_modules anymore. I am pretty sure it's a dependency of write-file-atomic.

@wraithgar
Copy link
Member

The breaking change commits for libnpmversion and libnpmpack need BREAKING CHANGE clauses.

@lukekarrys
Copy link
Contributor Author

I'm not sure what combination of install commands caused signal-exit to update, but in doing so it looks like it deleted two erroneous files that are currently present in node_modules/signal-exit but shouldn't be: https://github.com/npm/cli/tree/9622597399ec93224fddf90a9209a98dbcfd6b2f/node_modules/signal-exit

@wraithgar
Copy link
Member

LGTM. update the comment, rebase/squash, and I'll approve it. Thanks for taking it in steps w/ TODOS.

BREAKING CHANGE: libnpmexec now emits an output event on process
instead of invoking the output function passed in
BREAKING CHANGE: libnpmversion no longer takes a `silent` option to
suppress output from `@npmcli/run-script`. That output is now emitted
via an `output` event on `process`.
BREAKING CHANGE: libnpmpack no longer takes a `silent` option to
suppress output from `@npmcli/run-script`. That output is now emitted
via an `output` event on `process`.
@lukekarrys
Copy link
Contributor Author

@wraithgar I added BREAKING CHANGE notes, updated the comments, and rebased so commit linting should pass now.

@lukekarrys lukekarrys merged commit 9123de4 into latest Apr 16, 2024
224 of 225 checks passed
@lukekarrys lukekarrys deleted the lk/proc-log-deps branch April 16, 2024 15:58
@github-actions github-actions bot mentioned this pull request Apr 12, 2024
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.

None yet

2 participants