Skip to content

Conversation

@jamesgeorge007
Copy link
Contributor

@jamesgeorge007jamesgeorge007 commented Nov 20, 2018

Converted the es5 functions present within the test/pummel/test-net-pause.js to arrow (es6) functions which is more concise.

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

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Nov 20, 2018
@gireeshpunathilgireeshpunathil added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Nov 21, 2018
Trott
Trott previously requested changes Nov 21, 2018
@jamesgeorge007
Copy link
ContributorAuthor

@Trott What do you think now?

@gireeshpunathil
Copy link
Member

@gireeshpunathil
Copy link
Member

earlier CI run was aborted. new CI: https://ci.nodejs.org/job/node-test-pull-request/18879/

@addaleax
Copy link
Member

@jamesgeorge007 If you could rebase out the merge commit here, that could help – our CI doesn’t play well with those.

CI (rebasing disabled): https://ci.nodejs.org/job/node-test-pull-request/18890/

@jamesgeorge007
Copy link
ContributorAuthor

jamesgeorge007 commented Nov 23, 2018

@addaleax My working tree became dirty. Hence, I had to close the PR and reopen it again.
Hope everythings fine now 👍

@addaleax
Copy link
Member

@jamesgeorge007 Hm … I think the linter failure from https://travis-ci.com/nodejs/node/jobs/160538871 is real:

$ make lint make[1]: Entering directory `/home/travis/build/nodejs/node' Running JS linter... /home/travis/build/nodejs/node/test/pummel/test-net-pause.js 91:4 error Newline required at end of file but not found eol-last ✖ 1 problem (1 error, 0 warnings) 1 error and 0 warnings potentially fixable with the `--fix` option. 

@addaleax
Copy link
Member

@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 24, 2018
@jamesgeorge007jamesgeorge007force-pushed the patch-1 branch 2 times, most recently from 2947c85 to d32d64cCompareNovember 24, 2018 03:07
@Trott
Copy link
Member

@gireeshpunathil
Copy link
Member

@jamesgeorge007jamesgeorge007force-pushed the patch-1 branch 3 times, most recently from 17a9ef5 to b62fe77CompareNovember 24, 2018 16:53
@TrottTrott removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 25, 2018
@jamesgeorge007jamesgeorge007force-pushed the patch-1 branch 3 times, most recently from cdb949f to dee06f8CompareNovember 25, 2018 09:35
add newline at EOF 👍 Add EOF 👍 fix 👍 fix 👍 fix 👍
@jamesgeorge007jamesgeorge007 changed the title Convert functions to es6 styleChange callback to ES6 styleNov 25, 2018
@gireeshpunathil
Copy link
Member

@gireeshpunathil
Copy link
Member

landed as afab340

thank you @jamesgeorge007 for the contribution! Wish you great success with continued contribution to this project, if you are further interested please have a look at https://www.nodetodo.org/next-steps

gireeshpunathil pushed a commit that referenced this pull request Nov 25, 2018
PR-URL: #24513 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
@jamesgeorge007
Copy link
ContributorAuthor

👍

@gireeshpunathil
Copy link
Member

@gireeshpunathil What was the reason behind closing this PR instead of merging? Just curious

@jamesgeorge007 - don't worry; your code is landed as afab340 into the repo, and you have become a contributor in the project.

targos pushed a commit that referenced this pull request Nov 27, 2018
PR-URL: #24513 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
rvagg pushed a commit that referenced this pull request Nov 28, 2018
PR-URL: #24513 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
@BridgeARBridgeAR mentioned this pull request Dec 5, 2018
4 tasks
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
PR-URL: nodejs#24513 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 11, 2019
PR-URL: #24513 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Feb 12, 2019
rvagg pushed a commit that referenced this pull request Feb 28, 2019
PR-URL: #24513 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

11 participants

@jamesgeorge007@gireeshpunathil@addaleax@Trott@jasnell@cjihrig@BridgeAR@trivikr@refack@BethGriggs@nodejs-github-bot