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

test_runner: add jsdocs to MockFunctionContext and MockTracker #49555

Merged
merged 2 commits into from
Sep 14, 2023
Merged

test_runner: add jsdocs to MockFunctionContext and MockTracker #49555

merged 2 commits into from
Sep 14, 2023

Conversation

ocodista
Copy link
Contributor

@ocodista ocodista commented Sep 8, 2023

Add JSDocs for all public functions of MockFunctionContext

cc @nodejs/test_runner

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 8, 2023 •
edited by ErickWendel
Loading

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Sep 8, 2023
@ocodista ocodista changed the title test_runner: add jsdocs to MockFunctionContext test_runner: add jsdocs to MockFunctionContext and MockTracker Sep 8, 2023
ErickWendel
ErickWendel requested changes Sep 8, 2023
View reviewed changes
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
@ErickWendel
Copy link
Member

@ocodista Also, don't forget to add jsdocs for constructors, getters, and setters and everything that would be accessible on the public API

@ocodista
Copy link
Contributor Author

ocodista commented Sep 8, 2023

@ErickWendel Thank you for the notes and reviews, I applied all the naming suggestions and included jsdocs for the getter/setter and get times function. Please take a new look whenever you have some time.

@benjamingr
Copy link
Member

Hey thanks for your contribution 🙇 please revert all the unrelated style changes and keep the JSDoc stuff?

@ocodista
Copy link
Contributor Author

Hey thanks for your contribution 🙇 please revert all the unrelated style changes and keep the JSDoc stuff?

Thank you!

Just did it 😁

ErickWendel
ErickWendel approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@ErickWendel ErickWendel left a comment

Choose a reason for hiding this comment

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

LGTM

@ErickWendel ErickWendel added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 13, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 13, 2023
@nodejs-github-bot
Copy link
Collaborator

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

atlowChemi
atlowChemi approved these changes Sep 13, 2023
View reviewed changes
Copy link
Member

@atlowChemi atlowChemi left a comment

Choose a reason for hiding this comment

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

Overall LGTM, thanks for this contribution 🙂

lib/internal/test_runner/mock/mock.js Outdated Show resolved Hide resolved
@ocodista @atlowChemi
Update lib/internal/test_runner/mock/mock.js
794a162 
Co-authored-by: Chemi Atlow <chemi@atlow.co.il>
@atlowChemi atlowChemi added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 13, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 13, 2023
@nodejs-github-bot
Copy link
Collaborator

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

@nodejs-github-bot
Copy link
Collaborator

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

@github-actions github-actions bot mentioned this pull request Sep 14, 2023
Open
15 tasks
@ocodista
Copy link
Contributor Author

Overall LGTM, thanks for this contribution 🙂

Thank you!!!

@atlowChemi atlowChemi added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 14, 2023
@atlowChemi atlowChemi added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Sep 14, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 14, 2023
@nodejs-github-bot nodejs-github-bot merged commit 4efa374 into nodejs:main Sep 14, 2023
30 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 4efa374

This was referenced Sep 15, 2023
Open
Open
Open
Open
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@ocodista @ruyadorno
test_runner: add jsdocs to mock.js
5672e38 
PR-URL: #49555
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
This was referenced Sep 28, 2023
Closed
Merged
alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
@ocodista @alexfernandez
test_runner: add jsdocs to mock.js
4f56c6c 
PR-URL: nodejs#49555
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
targos pushed a commit that referenced this pull request Nov 27, 2023
@ocodista @targos
test_runner: add jsdocs to mock.js
76a3563 
PR-URL: #49555
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@targos targos mentioned this pull request Nov 28, 2023
Merged
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
test_runner: add jsdocs to mock.js
47b5f21 
PR-URL: nodejs/node#49555
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
sercher added a commit to sercher/graaljs that referenced this pull request Apr 25, 2024
@sercher
test_runner: add jsdocs to mock.js
5efedab 
PR-URL: nodejs/node#49555
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atlowChemi atlowChemi atlowChemi approved these changes

@ErickWendel ErickWendel ErickWendel approved these changes

Assignees
No one assigned
Labels
commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ocodista @nodejs-github-bot @ErickWendel @benjamingr @atlowChemi