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.

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

doc: fix/improve inspector profiler example #19379

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Mar 15, 2018

The first parameter to the callback is err. Fix that. Expand example
to demonstrate an actual write to disk.

Checklist

CI-lite: https://ci.nodejs.org/job/node-test-commit-lite/410/
https://ci.nodejs.org/job/node-test-commit-lite/433/

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. inspector Issues and PRs related to the V8 inspector protocol labels Mar 15, 2018
// write profile to disk, upload, etc.
fs.writeFileSync('./profile.cpuprofile', JSON.stringify(profile));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a policy on whether to include the require calls for common modules such as fs in sample code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a require of 'fs'.

// write profile to disk, upload, etc.
fs.writeFileSync('./profile.cpuprofile', JSON.stringify(profile));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the example use profile only if err is falsy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

The first parameter to the callback is `err`. Fix that. Expand example
to demonstrate an actual write to disk.

PR-URL: nodejs#19379
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@ofrobots ofrobots merged commit 2725acf into nodejs:master Mar 20, 2018
@ofrobots
Copy link
Contributor Author

Landed in 2725acf.

@ofrobots ofrobots deleted the inspector-example branch March 20, 2018 21:31
MylesBorins pushed a commit that referenced this pull request Mar 22, 2018
The first parameter to the callback is `err`. Fix that. Expand example
to demonstrate an actual write to disk.

PR-URL: #19379
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants