Skip to content

Conversation

@deokjinkim
Copy link
Contributor

@deokjinkim deokjinkim commented Dec 5, 2022

In document, autoSelectFamilyAttemptTimeout is described as positive integer because it's time unit. But there is no checking whether it's positive integer.

Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener

In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem. labels Dec 5, 2022
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

Could you add a test for this case?

Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

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

Thanks for spotting it! It looks good to me.
As @mcollina asked, can you please add a small test about it?

@deokjinkim deokjinkim force-pushed the 221205_check_positive branch from 1e767c8 to 9e89229 Compare December 5, 2022 10:36
@deokjinkim
Copy link
Contributor Author

Added test case for this PR. Please let me know if this PR needs change.

Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

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

LGTM!

@ShogunPanda ShogunPanda added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 6, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 6, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

ShogunPanda pushed a commit that referenced this pull request Dec 8, 2022
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@ShogunPanda
Copy link
Contributor

Landed in 3f98be9

@ShogunPanda ShogunPanda closed this Dec 8, 2022
ErickWendel pushed a commit to ErickWendel/node that referenced this pull request Dec 12, 2022
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: nodejs#45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Dec 12, 2022
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Dec 13, 2022
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
danielleadams pushed a commit that referenced this pull request Dec 30, 2022
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 3, 2023
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 4, 2023
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 5, 2023
In document, `autoSelectFamilyAttemptTimeout` is described as
positive integer because it's time unit. But there is no checking
whether it's positive integer.

PR-URL: #45740
Refs: https://github.com/nodejs/node/blob/main/doc/api/net.md#socketconnectoptions-connectlistener
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. net Issues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants