Skip to content

Conversation

@jmcgui05
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Oct 6, 2017
@mscdexmscdex added the cluster Issues and PRs related to the cluster subsystem. label Oct 6, 2017
@TrottTrott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 6, 2017
Copy link
Member

@lancelance 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 the contribution!

Minor nit, and this can be address when the PR lands, but be sure to check out the commit message guidelines. In this case, affected core subsystem would be test, and the first line of the commit should be less than 50 characters.

Copy link
Member

Choose a reason for hiding this comment

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

The line length exceeds 80 characters and has to be fixed. This could be done while landing as well though.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks @lance@BridgeAR , I have updated the commit message to reflect the limit, my apologies. Please let me know if there is anything else.

Copy link
Member

Choose a reason for hiding this comment

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

It would have been great if you would have also changed this to reflect the line length ^^

@jmcgui05jmcgui05force-pushed the process-disconnect-fix branch from 1ffb088 to d12b6ffCompareOctober 9, 2017 00:54
@jmcgui05jmcgui05force-pushed the process-disconnect-fix branch from d12b6ff to 94586ceCompareOctober 9, 2017 00:56
@BridgeARBridgeAR self-assigned this Oct 9, 2017
BridgeAR pushed a commit that referenced this pull request Oct 9, 2017
PR-URL: #15944 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@BridgeAR
Copy link
Member

Landed in b2e46c5

Thanks for the PR, and congratulations on becoming a Node.js Contributor 🎉 !

@BridgeARBridgeAR closed this Oct 9, 2017
MylesBorins pushed a commit that referenced this pull request Oct 11, 2017
PR-URL: #15944 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Oct 11, 2017
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
PR-URL: nodejs/node#15944 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 17, 2017
PR-URL: #15944 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Oct 17, 2017
MylesBorins pushed a commit that referenced this pull request Oct 18, 2017
PR-URL: #15944 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
PR-URL: #15944 Reviewed-By: Lance Ball <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Nov 3, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clusterIssues and PRs related to the cluster subsystem.code-and-learnIssues related to the Code-and-Learn events and PRs submitted during the events.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@jmcgui05@BridgeAR@lance@mscdex@MylesBorins@Trott@nodejs-github-bot