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 type 'string | Buffer' for process stdout/stderr #3329

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

robertbrignull
Copy link
Contributor

The intention here is to avoid use of any when logging process stdout/stderr. This data comes to us in chunks which we want to call toString() on. We can do that with type safety if we introduce a asString method (very open to other names).

The only issues are the null and undefined values. In this I've gone with returning human-readable values since we're printing this to a log, but we could return the empty string instead. In practice I expect these values to never actually happen.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@robertbrignull robertbrignull requested a review from a team February 7, 2024 17:57
@robertbrignull robertbrignull requested a review from a team as a code owner February 7, 2024 17:57
@robertbrignull robertbrignull force-pushed the robertbrignull/asString branch from 415c8a0 to 58a0619 Compare February 8, 2024 10:26
@robertbrignull robertbrignull changed the title Introduce asString to call toString() safely on process stdout/stderr Use type 'string | Buffer' for process stdout/stderr Feb 8, 2024
Copy link
Member

@koesie10 koesie10 left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

@robertbrignull robertbrignull merged commit bd823b5 into main Feb 8, 2024
32 checks passed
@robertbrignull robertbrignull deleted the robertbrignull/asString branch February 8, 2024 10:56
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.

2 participants