Conversation
The property is inherited from net.Server and was marked as deprecated there since v0.9.7.
doc/api/vm.md
Outdated
There was a problem hiding this comment.
This isn't correct. It should probably instead be: [sandbox[, options]]
doc/changelogs/CHANGELOG_V5.md
Outdated
There was a problem hiding this comment.
It's not strictly necessary, just for consistency.
There was a problem hiding this comment.
I'd prefer to avoid changes to the changelog unless necessary
There was a problem hiding this comment.
That sounds reasonable, fixed, PTAL.
|
cc @nodejs/documentation |
2a72a88 to
456b91f
Compare
sam-github
left a comment
There was a problem hiding this comment.
Other than that #13491 (comment) did not get quite addressed, this looks good to me.
456b91f to
0ec2d65
Compare
|
@sam-github My bad, I actually fixed it but dropped the commit when I fixed this comment. PTAL. |
oath.md: make order of properties consistent tls.md: remove spaces in getPeerCertificate signature tls.md: add deprecation notice to server.connections http.md: fix signature of request.end crypto.md: change crypto parameters to camelCase vm.md: add missing apostrophe vm.md: fix signature of vm.runInNewContext zlib.md: improve description of zlib.createXYZ PR-URL: #13491 Ref: #9538 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in e318c8a |
|
FWIW, I would not have squashed before landing, each commit fixed completely independent problems. |
|
Sorry, thought we had discussed that in IRC (@refack). |
|
so did I :-)
|
|
To put that statement into context:
Must have misinterpreted your messages (after that ⬆️), sorry. |
|
Don't worry, at least it landed with commit meta data, and I'm sure some of mine have missed that. Just for next time. |
I also misinterpreted 🤷♂️ |
oath.md: make order of properties consistent tls.md: remove spaces in getPeerCertificate signature tls.md: add deprecation notice to server.connections http.md: fix signature of request.end crypto.md: change crypto parameters to camelCase vm.md: add missing apostrophe vm.md: fix signature of vm.runInNewContext zlib.md: improve description of zlib.createXYZ PR-URL: #13491 Ref: #9538 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
oath.md: make order of properties consistent tls.md: remove spaces in getPeerCertificate signature tls.md: add deprecation notice to server.connections http.md: fix signature of request.end crypto.md: change crypto parameters to camelCase vm.md: add missing apostrophe vm.md: fix signature of vm.runInNewContext zlib.md: improve description of zlib.createXYZ PR-URL: #13491 Ref: #9538 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes most of the points suggested in #9538. Some have already been fixed, and I consider some of the others irrelevant. I am not sure about points 7, 8 and 16, though.
I did not notice another PR #9703 already worked on this without being merged until I pushed my commits. Apparently there was no activity for almost half a year, but feel free to close this PR if you think we should wait for the other one to get back to life.
Checklist
Affected core subsystem(s)
doc