Skip to content

assert: align argument names - #22760

Closed
BridgeAR wants to merge 2 commits into
nodejs:masterfrom
BridgeAR:improve-assert-block-name
Closed

assert: align argument names#22760
BridgeAR wants to merge 2 commits into
nodejs:masterfrom
BridgeAR:improve-assert-block-name

Conversation

@BridgeAR

@BridgeARBridgeAR commented Sep 8, 2018

Copy link
Copy Markdown
Member

This makes sure the documented argument names and the ones thrown
in errors is aligned with the actual argument name.

This addresses the comment from @Trott#22692 (comment).

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@BridgeAR
assert: align argument names
8ed2bc4
This makes sure the documented argument names and the ones thrown
in errors is aligned with the actual argument name.
@BridgeAR
BridgeAR requested a review from TrottSeptember 8, 2018 00:34
@nodejs-github-bot

nodejs-github-bot commented Sep 8, 2018

Copy link
Copy Markdown
Collaborator

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/831/pipeline

@nodejs-github-botnodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Sep 8, 2018
Trott
Trott approved these changes Sep 8, 2018
View reviewed changes

@TrottTrott 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. Thanks for the quick fix.

@BridgeAR

BridgeAR commented Sep 8, 2018

Copy link
Copy Markdown
MemberAuthor

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

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 8, 2018
lundibundi
lundibundi approved these changes Sep 8, 2018
View reviewed changes

@lundibundilundibundi 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. Nice reformatting 👍

Comment threadtest/parallel/test-net-connect-options-port.js Outdated
noop);
for (const block of portHostOptBlocks) {
block();
const portHostOptFns = doConnect([{ port: port, host: 'localhost' }], noop);

@lundibundilundibundiSep 8, 2018

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.

{ port, host: 'localhost' } ?

Comment threadtest/parallel/test-net-connect-options-port.js Outdated
dont);
for (const block of portHostOptBlocks) {
block().on('error', onError());
const portHostOptFns = doConnect([{ port: port, host: 'localhost' }], dont);

@lundibundilundibundiSep 8, 2018

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.

ditto.

@BridgeAR

BridgeAR commented Sep 9, 2018

Copy link
Copy Markdown
MemberAuthor

Comments addressed.

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

@BridgeAR

BridgeAR commented Sep 11, 2018•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Resumbed CI: https://ci.nodejs.org/job/node-test-commit/21362/ ✔️ (besides windows that is rerun afterwards)

@BridgeAR

BridgeAR commented Sep 11, 2018•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Rerunning Windows: https://ci.nodejs.org/job/node-test-commit-windows-fanned/20664/ ✔️

@BridgeAR

BridgeAR commented Sep 11, 2018

Copy link
Copy Markdown
MemberAuthor

Landed in b5430d7

BridgeAR added a commit to BridgeAR/node that referenced this pull request Sep 11, 2018
@BridgeAR
assert: align argument names
b5430d7
This makes sure the documented argument names and the ones thrown
in errors is aligned with the actual argument name.
PR-URL: nodejs#22760
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
targos pushed a commit that referenced this pull request Sep 12, 2018
@BridgeAR@targos
assert: align argument names
108a169
This makes sure the documented argument names and the ones thrown
in errors is aligned with the actual argument name.
PR-URL: #22760
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
@targostargos removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v10.x labels Sep 12, 2018
@targostargos mentioned this pull request Sep 18, 2018
Merged
targos pushed a commit that referenced this pull request Sep 19, 2018
@BridgeAR@targos
assert: align argument names
52877ee
This makes sure the documented argument names and the ones thrown
in errors is aligned with the actual argument name.
PR-URL: #22760
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
targos pushed a commit that referenced this pull request Sep 20, 2018
@BridgeAR@targos
assert: align argument names
0015430
This makes sure the documented argument names and the ones thrown
in errors is aligned with the actual argument name.
PR-URL: #22760
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
@BridgeAR
BridgeAR deleted the improve-assert-block-name branch January 20, 2020 11:38
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
@TrottTrottTrott approved these changes
@lpincalpincalpinca approved these changes
@trivikrtrivikrtrivikr approved these changes
+3 more reviewers
@hiroppyhiroppyhiroppy approved these changes
@lundibundilundibundilundibundi approved these changes
@mihalskiymihalskiymihalskiy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

assertIssues and PRs related to the assert subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@BridgeAR@nodejs-github-bot@jasnell@Trott@lpinca@hiroppy@lundibundi@trivikr@mihalskiy@targos