Skip to content

Conversation

@Flarna
Copy link
Member

@FlarnaFlarna commented Dec 6, 2023

tracingChannel.traceCallback() requires a callback otherwise it throws and invalid argument error. As a result arguments are not optional.

Correct the documentation to reflect that arguments are not optional.

Besides that correct description regarding signaling of errors.

Remove an unneeded null check in wrappedCallback() which can't happen because it's validated that callback is of type function.

Fixes: #50996

@nodejs-github-botnodejs-github-bot added diagnostics_channel Issues and PRs related to diagnostics channel needs-ci PRs that need a full CI run. labels Dec 6, 2023
@FlarnaFlarnaforce-pushed the gst/diag-channel-trace-callback branch from 481f145 to e1e19b5CompareDecember 6, 2023 07:48
@FlarnaFlarna requested a review from QardDecember 6, 2023 07:53
tracingChannel.traceCallback() requires a callback otherwise it throws and invalid argument error. As a result arguments are not optional. Correct the documentation to reflect that arguments are not optional. Besides that correct description regarding signaling of errors. Remove an unneeded null check in wrappedCallback() which can't happen because it's validated that callback is of type function.
@FlarnaFlarnaforce-pushed the gst/diag-channel-trace-callback branch from e1e19b5 to 1559cd8CompareDecember 6, 2023 07:58
Qard
Qard approved these changes Dec 6, 2023
Copy link
Member

@QardQard left a comment

Choose a reason for hiding this comment

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

Other than typo, LGTM.

Co-authored-by: Stephen Belanger <[email protected]>
Qard
Qard approved these changes Dec 6, 2023
@FlarnaFlarna added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 6, 2023
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 6, 2023
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/56128/

@FlarnaFlarna added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 7, 2023
@FlarnaFlarna added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Dec 8, 2023
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 8, 2023
@nodejs-github-botnodejs-github-bot merged commit 3f942e2 into nodejs:mainDec 8, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 3f942e2

@FlarnaFlarna deleted the gst/diag-channel-trace-callback branch December 8, 2023 22:26
RafaelGSS pushed a commit that referenced this pull request Dec 15, 2023
tracingChannel.traceCallback() requires a callback otherwise it throws and invalid argument error. As a result arguments are not optional. Correct the documentation to reflect that arguments are not optional. Besides that correct description regarding signaling of errors. Remove an unneeded null check in wrappedCallback() which can't happen because it's validated that callback is of type function. PR-URL: #51068Fixes: #50996 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
@RafaelGSSRafaelGSS mentioned this pull request Dec 15, 2023
richardlau pushed a commit that referenced this pull request Mar 25, 2024
tracingChannel.traceCallback() requires a callback otherwise it throws and invalid argument error. As a result arguments are not optional. Correct the documentation to reflect that arguments are not optional. Besides that correct description regarding signaling of errors. Remove an unneeded null check in wrappedCallback() which can't happen because it's validated that callback is of type function. PR-URL: #51068Fixes: #50996 Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
@richardlaurichardlau mentioned this pull request Mar 25, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.diagnostics_channelIssues and PRs related to diagnostics channelneeds-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[diagnostics_channel] tracingChannel.traceCallback incorrect types

5 participants

@Flarna@nodejs-github-bot@Qard@lpinca@VoltrexKeyva