Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc: replace Function with Arrow Function #17459

Closed
wants to merge 1 commit into from
Closed

doc: replace Function with Arrow Function #17459

wants to merge 1 commit into from

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

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

@mithunsasidharan
Copy link
Contributor Author

@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

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

@apapirovski
Copy link
Member

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

@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

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

@mithunsasidharan
Copy link
Contributor Author

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

@apapirovski
Copy link
Member

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

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

is 7f9d65f72e923637ad28cfc6adf338f37e323a61 correct or were there more changes?

@mithunsasidharan
Copy link
Contributor Author

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

@mithunsasidharan
Copy link
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
Copy link
Member

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

@maclover7 maclover7 maclover7 approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

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