FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix #30068: refactored to use arrow functions by mpark86 · Pull Request #30069 · nodejs/node · GitHub

/ node Public

Fix #30068: refactored to use arrow functions - #30069

Closed
mpark86 wants to merge 1 commit into
nodejs:masterfrom
mpark86:issue-30068
ConversationCommits (1)ChecksFiles changed
Closed

Fix #30068: refactored to use arrow functions#30069
mpark86 wants to merge 1 commit into
nodejs:masterfrom
mpark86:issue-30068

Conversation

mpark86 commented Oct 22, 2019

Copy link
Copy Markdown
Contributor

Modified common.mustCall() to use arrow functions.

nodejs-github-bot added addons Issues and PRs related to native addons. test Issues and PRs related to the tests. labels Oct 22, 2019

nodejs-github-bot commented Oct 22, 2019 •
edited by Trott
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/26152/ ✔️

Trott added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Oct 22, 2019

gireeshpunathil commented Oct 23, 2019

Copy link
Copy Markdown
Member

@mpark86 - the build failure is due to the error here:
https://travis-ci.com/nodejs/node/jobs/248453662#L259
Invalid subsystem: "Fix #30068" - the commit message should follow a guideline (the message should start with a valid subsystem, followed by a colon, followed by a space and then followed by a short description, altogether less than 52 characters)

your commit message can be:
test: refactor to use arrow functions

mpark86 commented Oct 24, 2019

Copy link
Copy Markdown
Contributor Author

@gireeshpunathil Oh okay. Thanks for letting me know. How would you recommend to do it? Is there a way to change the commit message for pushed changes?

gireeshpunathil commented Oct 24, 2019

Copy link
Copy Markdown
Member

@mpark86 - here are the steps:

  • git rebase -i
  • this will open up an editor for modifying your commit // default editor is vi I guess
  • you should see one line at the top as below, everything else should be comments
  • pick fd0b250 Fix #30068: refactored to use arrow functions
  • change pick to reword // make sure the space in between is preserved
  • save and quit // I usually do :wq
  • the editor opens up again, this time with the commit message opened up for editing
  • change it to test: refactor to use arrow functions
  • save and quit again
  • issue git push -f origin issue-30068

Trott pushed a commit that referenced this pull request Oct 25, 2019
test: use arrow functions for callbacks
175c318 
Use arrow functions for callbacks in
test/addons/make-callback-recurse/test.js.

PR-URL: #30069
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

Trott commented Oct 25, 2019

Copy link
Copy Markdown
Member

Landed in 175c318

Trott closed this Oct 25, 2019

Trott commented Oct 25, 2019

Copy link
Copy Markdown
Member

Thanks for the contribution! 🎉

targos pushed a commit that referenced this pull request Oct 28, 2019
test: use arrow functions for callbacks
1d9f427 
Use arrow functions for callbacks in
test/addons/make-callback-recurse/test.js.

PR-URL: #30069
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

mpark86 commented Oct 28, 2019

Copy link
Copy Markdown
Contributor Author

Thank you. It was good to contribute to Node.js project!

targos mentioned this pull request Nov 5, 2019
Merged
targos pushed a commit that referenced this pull request Nov 8, 2019
test: use arrow functions for callbacks
5191f0f 
Use arrow functions for callbacks in
test/addons/make-callback-recurse/test.js.

PR-URL: #30069
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this pull request Nov 10, 2019
test: use arrow functions for callbacks
4a740d5 
Use arrow functions for callbacks in
test/addons/make-callback-recurse/test.js.

PR-URL: #30069
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos pushed a commit that referenced this pull request Nov 10, 2019
test: use arrow functions for callbacks
bc03fb3 
Use arrow functions for callbacks in
test/addons/make-callback-recurse/test.js.

PR-URL: #30069
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
targos mentioned this pull request Nov 10, 2019
Merged
targos pushed a commit that referenced this pull request Nov 11, 2019
test: use arrow functions for callbacks
d96e8b6 
Use arrow functions for callbacks in
test/addons/make-callback-recurse/test.js.

PR-URL: #30069
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Trott Trott approved these changes
addaleax addaleax approved these changes
benjamingr benjamingr approved these changes
gireeshpunathil gireeshpunathil approved these changes
trivikr trivikr approved these changes
+1 more reviewer
Cretezy Cretezy approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

addons Issues and PRs related to native addons. author ready PRs that have at least one approval, no outstanding review comments, and a CI started. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL