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: replace dead link in v8 module #14372

Closed
wants to merge 2 commits into from

Conversation

drboyer
Copy link
Contributor

@drboyer drboyer commented Jul 19, 2017

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-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jul 19, 2017
@vsemozhetbyt vsemozhetbyt added the v8 engine Issues and PRs related to the V8 dependency. label Jul 19, 2017
Copy link
Member

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

@cjihrig cjihrig 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
Contributor Author

drboyer commented Jul 19, 2017

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 <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@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 <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Jul 24, 2017
PR-URL: #14372
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@addaleax addaleax mentioned this pull request Jul 24, 2017
@addaleax addaleax 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 <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins 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 <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 3, 2017
PR-URL: #14372
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
PR-URL: #14372
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@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. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants