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

doc: replace Function with Arrow Function by mithunsasidharan · Pull Request #17459 · nodejs/node · GitHub

/ node Public

doc: replace Function with Arrow Function - #17459

Closed
mithunsasidharan wants to merge 1 commit into
nodejs:masterfrom
mithunsasidharan:pr_doc
ConversationCommits (1)ChecksFiles changed
Closed

doc: replace Function with Arrow Function#17459
mithunsasidharan wants to merge 1 commit into
nodejs:masterfrom
mithunsasidharan:pr_doc

Conversation

mithunsasidharan commented Dec 4, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Replaced classic functions with arrow functions inside doc/api/util.md and doc/api/vm.md

Checklist
Affected core subsystem(s)

doc

nodejs-github-bot added doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module. labels Dec 4, 2017

maclover7 commented Dec 5, 2017

Copy link
Copy Markdown
Contributor

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

mithunsasidharan commented Dec 5, 2017

Copy link
Copy Markdown
Contributor Author

@maclover7 : Curious why test/linux-one failed ?

BridgeAR commented Dec 6, 2017

Copy link
Copy Markdown
Member

I would say this can land below the 48 hours.

BridgeAR commented Dec 6, 2017

Copy link
Copy Markdown
Member

@mithunsasidharan the tests are some times flaky. This has nothing to with that your PR.

maclover7 added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 6, 2017

mithunsasidharan commented Dec 6, 2017

Copy link
Copy Markdown
Contributor Author

@apapirovski : Can you please land this ? Thanks a lot !

apapirovski commented Dec 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hmm... strangely this doesn't seem to apply cleanly as a patch. Could you rebase against master by any chance and force push?

maclover7 commented Dec 6, 2017

Copy link
Copy Markdown
Contributor

@mithunsasidharan Please try and avoid pinging to land things -- since it already has the ready label, it should be landed soon. There are almost 200 PRs open in nodejs/node, and everyone pinging would lead to a lot of notifications.

mithunsasidharan commented Dec 6, 2017

Copy link
Copy Markdown
Contributor Author

@apapirovski : I've updated the branch.. kindly check now.. !

mithunsasidharan commented Dec 6, 2017

Copy link
Copy Markdown
Contributor Author

@maclover7 : Sorry about it. Will follow the guidelines here on. Thanks much !

apapirovski commented Dec 6, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

@mithunsasidharan Looks like there's a merge commit here. You could do something like

git reset --hard origin/master
git cherry-pick d4c98e47835d22eba7147fa73068983fc8d646fd

But I'm no git surgeon. Maybe there's a better way to get rid of it.

Edit: could also be upstream/master instead of origin/master if you have a remote reference to upstream.

mithunsasidharan commented Dec 6, 2017

Copy link
Copy Markdown
Contributor Author

@apapirovski : I'm sorry.. seems I've messed up the branch... will it be possible for you to land specific commit (cherrypick) ? Thanks!

apapirovski commented Dec 6, 2017

Copy link
Copy Markdown
Contributor

is 7f9d65f72e923637ad28cfc6adf338f37e323a61 correct or were there more changes?

mithunsasidharan commented Dec 6, 2017

Copy link
Copy Markdown
Contributor Author

@apapirovski : Give me 2 minutes.. Il fix this!

mithunsasidharan commented Dec 6, 2017

Copy link
Copy Markdown
Contributor Author

@apapirovski : There you go.. corrected the PR and commits.. there was 1 more file but seems its resolved in previous PR. So just 1 file and 1 commit.. please go ahead with the landing ! Thanks for your support !

apapirovski commented Dec 6, 2017

Copy link
Copy Markdown
Contributor

Landed in 817d4ad

apapirovski closed this Dec 6, 2017
apapirovski pushed a commit that referenced this pull request Dec 6, 2017
doc: use arrow functions in util.md sample code
817d4ad 
PR-URL: #17459
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
addaleax removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Dec 7, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
doc: use arrow functions in util.md sample code
346f387 
PR-URL: #17459
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
doc: use arrow functions in util.md sample code
7e38821 
PR-URL: #17459
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins mentioned this pull request Dec 12, 2017
Merged
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

jasnell jasnell approved these changes
lpinca lpinca approved these changes
BridgeAR BridgeAR approved these changes
+1 more reviewer
maclover7 maclover7 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. util Issues and PRs related to the built-in util module.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL