Skip to content

lib: add JSDoc typings for assert - #38188

Closed
VoltrexKeyva wants to merge 7 commits into
nodejs:masterfrom
VoltrexKeyva:patch-3
Closed

lib: add JSDoc typings for assert#38188
VoltrexKeyva wants to merge 7 commits into
nodejs:masterfrom
VoltrexKeyva:patch-3

Conversation

@VoltrexKeyva

@VoltrexKeyvaVoltrexKeyva commented Apr 10, 2021

Copy link
Copy Markdown
Contributor

Added JSDoc typings for the assert lib.

@VoltrexKeyva
lib: add JSDoc typings for assert
3f89ecb
Added JSDoc typings for the assert lib.
@nodejs-github-botnodejs-github-bot added assert Issues and PRs related to the assert subsystem. needs-ci PRs that need a full CI run. labels Apr 10, 2021
@VoltrexKeyva
Fix ifError() description typo
066aaae
Fixed the typo `of` -> `is` of the `ifError()` method's JSDoc description.
bmeck
bmeck reviewed Apr 11, 2021
View reviewed changes
Comment threadlib/assert.js Outdated
bmeck
bmeck reviewed Apr 11, 2021
View reviewed changes
Comment threadlib/assert.js Outdated
VoltrexKeyvaand others added 2 commits April 11, 2021 06:12
@VoltrexKeyva@bmeck
Change return type
c8566ac
void -> never.
Co-authored-by: Bradley Farias <bradley.meck@gmail.com>
@VoltrexKeyva@bmeck
Change function type
ac88cb4
Change `Function` -> `() => any`.
Co-authored-by: Bradley Farias <bradley.meck@gmail.com>
bmeck
bmeck approved these changes Apr 12, 2021
View reviewed changes

@bmeckbmeck left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, nits we can improve later if desired

Comment threadlib/assert.js Outdated
Comment threadlib/assert.js Outdated
Comment threadlib/assert.js Outdated
@VoltrexKeyva
Improve function types
a90d6e1
Improved function types by making it indicate what the function's value should be in the parameters.
targos
targos reviewed Apr 13, 2021
View reviewed changes
Comment threadlib/assert.js Outdated
Comment threadlib/assert.js Outdated
@VoltrexKeyva
Promise return type
79c37da
Some of the functions returned a promise instead of straight up `void`, `void` -> `Promise<void>`.
@VoltrexKeyva
Reduce line length
97639f3
Done for checks.
@VoltrexKeyva

VoltrexKeyva commented Apr 14, 2021

Copy link
Copy Markdown
ContributorAuthor

Why does the CI even fail :suspect:

@nodejs-github-bot

nodejs-github-bot commented Apr 14, 2021

Copy link
Copy Markdown
Collaborator

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

@targostargos self-assigned this Apr 14, 2021
@nodejs-github-bot

nodejs-github-bot commented Apr 14, 2021

Copy link
Copy Markdown
Collaborator

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

@targos

targos commented Apr 15, 2021

Copy link
Copy Markdown
Member

Landed in 82d5988

@targostargos closed this Apr 15, 2021
targos pushed a commit that referenced this pull request Apr 15, 2021
@VoltrexKeyva@targos
typings: add JSDoc typings for assert
82d5988
PR-URL: #38188
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@VoltrexKeyva
VoltrexKeyva deleted the patch-3 branch April 15, 2021 14:46
@BethGriggsBethGriggs mentioned this pull request Apr 16, 2021
Merged
targos pushed a commit that referenced this pull request May 1, 2021
@VoltrexKeyva@targos
typings: add JSDoc typings for assert
530e69e
PR-URL: #38188
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@danielleadamsdanielleadams mentioned this pull request May 3, 2021
Merged
danielleadams pushed a commit that referenced this pull request May 8, 2021
@VoltrexKeyva@danielleadams
typings: add JSDoc typings for assert
fee39ea
PR-URL: #38188
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@targostargos removed their assignment Oct 31, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@targostargostargos approved these changes
+1 more reviewer
@bmeckbmeckbmeck approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

assertIssues and PRs related to the assert subsystem.needs-ciPRs that need a full CI run.typings

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@VoltrexKeyva@nodejs-github-bot@targos@bmeck