Skip to content

doc: replace Function with Arrow Function#17459

Closed
mithunsasidharan wants to merge 1 commit intonodejs:masterfrom
mithunsasidharan:pr_doc
Closed

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

Conversation

@mithunsasidharan
Copy link
Contributor

@mithunsasidharan mithunsasidharan commented Dec 4, 2017 •
edited
Loading

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 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
Copy link
Contributor

maclover7 commented Dec 5, 2017

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

@mithunsasidharan
Copy link
Contributor Author

mithunsasidharan commented Dec 5, 2017

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

@BridgeAR
Copy link
Member

BridgeAR commented Dec 6, 2017

I would say this can land below the 48 hours.

@BridgeAR
Copy link
Member

BridgeAR commented Dec 6, 2017

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

@maclover7 maclover7 added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 6, 2017
@mithunsasidharan
Copy link
Contributor Author

mithunsasidharan commented Dec 6, 2017

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

@apapirovski
Copy link
Contributor

apapirovski commented Dec 6, 2017 •
edited
Loading

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

@maclover7
Copy link
Contributor

maclover7 commented Dec 6, 2017

@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
Copy link
Contributor Author

mithunsasidharan commented Dec 6, 2017

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

@mithunsasidharan
Copy link
Contributor Author

mithunsasidharan commented Dec 6, 2017

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

@apapirovski
Copy link
Contributor

apapirovski commented Dec 6, 2017 •
edited
Loading

@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
Copy link
Contributor Author

mithunsasidharan commented Dec 6, 2017

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

@apapirovski
Copy link
Contributor

apapirovski commented Dec 6, 2017

is 7f9d65f72e923637ad28cfc6adf338f37e323a61 correct or were there more changes?

@mithunsasidharan
Copy link
Contributor Author

mithunsasidharan commented Dec 6, 2017

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

@mithunsasidharan
Copy link
Contributor Author

mithunsasidharan commented Dec 6, 2017

@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
Copy link
Contributor

apapirovski commented Dec 6, 2017

Landed in 817d4ad

@apapirovski apapirovski closed this Dec 6, 2017
apapirovski pushed a commit that referenced this pull request Dec 6, 2017
@mithunsasidharan @apapirovski
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 addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 7, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@mithunsasidharan @MylesBorins
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
@mithunsasidharan @MylesBorins
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 MylesBorins mentioned this pull request Dec 12, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@lpinca lpinca lpinca approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

+1 more reviewer

@maclover7 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

@mithunsasidharan @maclover7 @BridgeAR @apapirovski @jasnell @lpinca @addaleax @gibfahn @nodejs-github-bot