Skip to content

Conversation

@drboyer
Copy link
Contributor

Identified a dead link to the V8 GetHeapSpaceStatistics() function on the v8 module documentation page. I updated the link to the version from the 6.10 version of the V8 docs, but could easily update it to the 8.0 version if desired, for some reason.

Checklist
Affected core subsystem(s)

doc

@nodejs-github-botnodejs-github-bot added the doc Issues and PRs related to the documentations. label Jul 19, 2017
@vsemozhetbytvsemozhetbyt added the v8 engine Issues and PRs related to the V8 dependency. label Jul 19, 2017
Copy link
Member

@joyeecheungjoyeecheung left a comment

Choose a reason for hiding this comment

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

LGTM as it is, using the 8.x version would be better but then the doc already says this data structure would change depending on v8 versions and there is more than one year until 6.x ends LTS.

Copy link
Contributor

@cjihrigcjihrig left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd update this to the Node 8 link, and update it to the Node 6 link when it's backported.

@drboyer
Copy link
ContributorAuthor

Thanks! I just updated the link to node-8.0 instead.

Is there anything further I need to do with this? (This is my first PR to the node.js project! 🎉)

joyeecheung pushed a commit that referenced this pull request Jul 19, 2017
PR-URL: #14372 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@joyeecheung
Copy link
Member

joyeecheung commented Jul 19, 2017

I believe this does not need to wait for 48 hours and we already got enough approvals. Landed in 7bc666b, thanks for your contribution and welcome to Node.js!

addaleax pushed a commit that referenced this pull request Jul 22, 2017
PR-URL: #14372 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Jul 24, 2017
PR-URL: #14372 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@addaleaxaddaleax mentioned this pull request Jul 24, 2017
@addaleaxaddaleax mentioned this pull request Aug 2, 2017
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
PR-URL: #14372 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Aug 16, 2017
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
PR-URL: #14372 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 3, 2017
PR-URL: #14372 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
PR-URL: #14372 Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@drboyer@joyeecheung@lpinca@cjihrig@MylesBorins@vsemozhetbyt@nodejs-github-bot