Skip to content

Conversation

@jasnell
Copy link
Member

@jasnell jasnell commented Nov 29, 2024

No description provided.

@jasnell jasnell requested review from anonrig and tniessen November 29, 2024 16:25
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Nov 29, 2024
@nodejs-github-bot

This comment was marked as outdated.

@jasnell jasnell requested a review from theanarkh November 29, 2024 16:27
@anonrig anonrig added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 29, 2024
@jasnell jasnell added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Nov 29, 2024
@codecov

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

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

/**
* Returns true if the value is a BlockList
* @param {any} value
* @returns {boolean}
Copy link
Member

Choose a reason for hiding this comment

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

I think we could use asserts here

Suggested change
* @returns {boolean}
* @returns {asserts value is BlockList}

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'm not quite sure what this would accomplish. The method does not assert at all, it just returns true or false and should not throw any errors.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

@jasnell
Copy link
Member Author

jasnell commented Dec 4, 2024

Landed in c7de0ec

@jasnell jasnell closed this Dec 4, 2024
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. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants