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.

Sign up for GitHub

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

Jump to bottom

doc: add emit to NodeEventTarget #46356

Closed
wants to merge 2 commits into from
Closed

doc: add emit to NodeEventTarget #46356

wants to merge 2 commits into from

Conversation

deokjinkim
Copy link
Contributor

NodeEventTarget.emit() is not described in document. Plus, make type parameter of removeAllListeners as optional.

Refs: #35851

@deokjinkim
doc: add emit to NodeEventTarget
81a7ffe 
NodeEventTarget.emit() is not described in document. Plus, make
type parameter of removeAllListeners as optional.

Refs: nodejs#35851
@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jan 26, 2023
@deokjinkim
Copy link
Contributor Author

/cc @nodejs/events

@lpinca lpinca added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jan 28, 2023
jasnell
jasnell reviewed Jan 29, 2023
View reviewed changes
doc/api/events.md Outdated Show resolved Hide resolved
@deokjinkim @jasnell
remove unnecessary empty lines
fd5e5d4 
Co-authored-by: James M Snell <jasnell@gmail.com>
@deokjinkim
Copy link
Contributor Author

@benjamingr Could you please review this PR because this PR adds document for API you implemented?

@benjamingr
Copy link
Member

@deokjinkim first: feel free to ping me for review whenever :)

Second: NodeEventTarget was @jasnell and a bit of @addaleax , I was a distant third (I did work a bunch on EventTarget just not those parts).

@deokjinkim deokjinkim added doc Issues and PRs related to the documentations. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 10, 2023
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Feb 10, 2023
@github-actions
Copy link
Contributor

Failed to start CI
- Validating Jenkins credentials
✖  Jenkins credentials invalid
https://github.com/nodejs/node/actions/runs/4139956554

@deokjinkim deokjinkim added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Feb 10, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 16, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Feb 18, 2023
Open
32 tasks
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@deokjinkim deokjinkim removed the needs-ci PRs that need a full CI run. label Feb 20, 2023
deokjinkim added a commit that referenced this pull request Feb 20, 2023
@deokjinkim
doc: add emit to NodeEventTarget
ac7ef31 
NodeEventTarget.emit() is not described in document. Plus, make
type parameter of removeAllListeners as optional.

Refs: #35851
PR-URL: #46356
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@deokjinkim
Copy link
Contributor Author

Landed in ac7ef31

@deokjinkim deokjinkim closed this Feb 20, 2023
This was referenced Feb 21, 2023
Open
Open
targos pushed a commit that referenced this pull request Mar 13, 2023
@deokjinkim @targos
doc: add emit to NodeEventTarget
dd66c48 
NodeEventTarget.emit() is not described in document. Plus, make
type parameter of removeAllListeners as optional.

Refs: #35851
PR-URL: #46356
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@targos targos mentioned this pull request Mar 14, 2023
Merged
danielleadams pushed a commit that referenced this pull request Apr 11, 2023
@deokjinkim @danielleadams
doc: add emit to NodeEventTarget
45fccc9 
NodeEventTarget.emit() is not described in document. Plus, make
type parameter of removeAllListeners as optional.

Refs: #35851
PR-URL: #46356
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@danielleadams danielleadams mentioned this pull request Apr 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjamingr benjamingr benjamingr approved these changes

@lpinca lpinca lpinca approved these changes

@aymen94 aymen94 aymen94 approved these changes

@jasnell jasnell Awaiting requested review from jasnell

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@deokjinkim @benjamingr @nodejs-github-bot @jasnell @lpinca @aymen94