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

typings: improve typings #40222

Closed
wants to merge 5 commits into from
Closed

typings: improve typings #40222

wants to merge 5 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Sep 26, 2021

typings: fix declaration of primordials
Closes: https://github.com/nodejs/node/issues/40144

typings: define types for timers binding

typings: add missing types to options and util bindings

typings: define types for os binding

@targos targos added the typings label Sep 26, 2021
getUptime(): number;
getTotalMem(): number;
getFreeMem(): number;
getCPUs(): Array<string | number>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
getCPUs(): Array<string | number>;
getCPUs(): (string | number)[];

for consistency with other dts

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a preference to use type[] for simple types and Array<types> for more complex ones

getTotalMem(): number;
getFreeMem(): number;
getCPUs(): Array<string | number>;
getInterfaceAddresses(ctx: object): Array<string | number | boolean> | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
getInterfaceAddresses(ctx: object): Array<string | number | boolean> | undefined;
getInterfaceAddresses(ctx: object): (string | number | boolean)[] | undefined;

typings/internalBinding/os.d.ts Outdated Show resolved Hide resolved
typings/internalBinding/os.d.ts Outdated Show resolved Hide resolved
@VoltrexKeyva VoltrexKeyva added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 30, 2021
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 9, 2021
@github-actions github-actions bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 9, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2021

Commit Queue failed
- Loading data for nodejs/node/pull/40222
✔  Done loading data for nodejs/node/pull/40222
----------------------------------- PR info ------------------------------------
Title      typings: improve typings (#40222)
Author     Michaël Zasso  (@targos)
Branch     targos:typings -> nodejs:master
Labels     author ready, typings
Commits    5
 - typings: fix declaration of primordials
 - typings: define types for timers binding
 - typings: add missing types to options and util bindings
 - typings: define types for os binding
 - fixup! typings: define types for os binding
Committers 1
 - Michaël Zasso 
PR-URL: https://github.com/nodejs/node/pull/40222
Fixes: https://github.com/https://github.com/nodejs/node/issues/40144
Reviewed-By: Tobias Nießen 
Reviewed-By: Evan Lucas 
Reviewed-By: Zijian Liu 
Reviewed-By: James M Snell 
Reviewed-By: Michael Dawson 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/40222
Fixes: https://github.com/https://github.com/nodejs/node/issues/40144
Reviewed-By: Tobias Nießen 
Reviewed-By: Evan Lucas 
Reviewed-By: Zijian Liu 
Reviewed-By: James M Snell 
Reviewed-By: Michael Dawson 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 26 Sep 2021 14:36:03 GMT
   ✔  Approvals: 5
   ✔  - Tobias Nießen (@tniessen) (TSC): https://github.com/nodejs/node/pull/40222#pullrequestreview-763706951
   ✔  - Evan Lucas (@evanlucas): https://github.com/nodejs/node/pull/40222#pullrequestreview-764570178
   ✔  - Zijian Liu (@Lxxyx): https://github.com/nodejs/node/pull/40222#pullrequestreview-766148910
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/40222#pullrequestreview-767943198
   ✔  - Michael Dawson (@mhdawson) (TSC): https://github.com/nodejs/node/pull/40222#pullrequestreview-769403509
   ✔  Last GitHub Actions successful
   ℹ  Green GitHub Actions CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/master up to date...
From https://github.com/nodejs/node
 * branch                  master     -> FETCH_HEAD
✔  origin/master is now up-to-date
- Downloading patch for 40222
From https://github.com/nodejs/node
 * branch                  refs/pull/40222/merge -> FETCH_HEAD
✔  Fetched commits as eaa59571e0fd..795db39025ef
--------------------------------------------------------------------------------
[master 0b4ed8fe7f] typings: fix declaration of primordials
 Author: Michaël Zasso 
 Date: Sun Sep 26 12:22:11 2021 +0200
 1 file changed, 5 insertions(+), 1 deletion(-)
[master c5028c4cfb] typings: define types for timers binding
 Author: Michaël Zasso 
 Date: Sun Sep 26 15:47:14 2021 +0200
 2 files changed, 9 insertions(+)
 create mode 100644 typings/internalBinding/timers.d.ts
[master 7b025d4363] typings: add missing types to options and util bindings
 Author: Michaël Zasso 
 Date: Sun Sep 26 16:07:13 2021 +0200
 2 files changed, 2 insertions(+)
[master 03be0fbe11] typings: define types for os binding
 Author: Michaël Zasso 
 Date: Sun Sep 26 16:28:40 2021 +0200
 2 files changed, 22 insertions(+)
 create mode 100644 typings/internalBinding/os.d.ts
[master 88e9b9da17] fixup! typings: define types for os binding
 Author: Michaël Zasso 
 Date: Mon Sep 27 16:50:49 2021 +0200
 1 file changed, 7 insertions(+), 7 deletions(-)
   ✔  Patches applied
There are 5 commits in the PR. Attempting autorebase.
Rebasing (2/9)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
typings: fix declaration of primordials

Closes: #40144

PR-URL: #40222
Fixes: #40144
Reviewed-By: Tobias Nießen [email protected]
Reviewed-By: Evan Lucas [email protected]
Reviewed-By: Zijian Liu [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Michael Dawson [email protected]

[detached HEAD 410ea3f9af] typings: fix declaration of primordials
Author: Michaël Zasso [email protected]
Date: Sun Sep 26 12:22:11 2021 +0200
1 file changed, 5 insertions(+), 1 deletion(-)
Rebasing (3/9)
Rebasing (4/9)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
typings: define types for timers binding

PR-URL: #40222
Fixes: #40144
Reviewed-By: Tobias Nießen [email protected]
Reviewed-By: Evan Lucas [email protected]
Reviewed-By: Zijian Liu [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Michael Dawson [email protected]

[detached HEAD 71c9083927] typings: define types for timers binding
Author: Michaël Zasso [email protected]
Date: Sun Sep 26 15:47:14 2021 +0200
2 files changed, 9 insertions(+)
create mode 100644 typings/internalBinding/timers.d.ts
Rebasing (5/9)
Rebasing (6/9)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
typings: add missing types to options and util bindings

PR-URL: #40222
Fixes: #40144
Reviewed-By: Tobias Nießen [email protected]
Reviewed-By: Evan Lucas [email protected]
Reviewed-By: Zijian Liu [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Michael Dawson [email protected]

[detached HEAD 56184dad50] typings: add missing types to options and util bindings
Author: Michaël Zasso [email protected]
Date: Sun Sep 26 16:07:13 2021 +0200
2 files changed, 2 insertions(+)
Rebasing (7/9)
Rebasing (8/9)
Rebasing (9/9)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
typings: define types for os binding

PR-URL: #40222
Fixes: #40144
Reviewed-By: Tobias Nießen [email protected]
Reviewed-By: Evan Lucas [email protected]
Reviewed-By: Zijian Liu [email protected]
Reviewed-By: James M Snell [email protected]
Reviewed-By: Michael Dawson [email protected]

[detached HEAD 2286b9fc7a] typings: define types for os binding
Author: Michaël Zasso [email protected]
Date: Sun Sep 26 16:28:40 2021 +0200
2 files changed, 22 insertions(+)
create mode 100644 typings/internalBinding/os.d.ts

Successfully rebased and updated refs/heads/master.
✖ 410ea3f9af075b40679d532d808a1a7d249e0455
✖ 4:7 Fixes must be a GitHub URL. fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 3:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length
✖ 71c9083927b2c4f8c4a15c4bcd306d864bf77c65
✖ 2:7 Fixes must be a GitHub URL. fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 1:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length
✖ 56184dad5073aa40ebb52a9faea80e33e852903b
✖ 2:7 Fixes must be a GitHub URL. fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 1:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
⚠ 0:50 Title should be <= 50 columns. title-length
✖ 2286b9fc7a5cc65aa97b02a895c335e1937f5172
✖ 2:7 Fixes must be a GitHub URL. fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 1:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length
ℹ Please fix the commit message and try again.

https://github.com/nodejs/node/actions/runs/1323958359

@targos targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Oct 10, 2021
@targos
Copy link
Member Author

targos commented Oct 10, 2021

Landed in 8068f40...9467cba

@targos targos closed this Oct 10, 2021
@targos targos deleted the typings branch October 10, 2021 09:04
targos added a commit that referenced this pull request Oct 10, 2021
PR-URL: #40222
Fixes: #40144
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos added a commit that referenced this pull request Oct 10, 2021
PR-URL: #40222
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos added a commit that referenced this pull request Oct 10, 2021
PR-URL: #40222
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos added a commit that referenced this pull request Oct 10, 2021
PR-URL: #40222
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos added a commit that referenced this pull request Oct 13, 2021
PR-URL: #40222
Fixes: #40144
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos added a commit that referenced this pull request Oct 13, 2021
PR-URL: #40222
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos added a commit that referenced this pull request Oct 13, 2021
PR-URL: #40222
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
targos added a commit that referenced this pull request Oct 13, 2021
PR-URL: #40222
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Zijian Liu <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
@richardlau richardlau mentioned this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. typings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants