doc: improve addons.markdown copy#4320
Uh oh!
There was an error while loading. Please reload this page.
Conversation
jasnell commented Dec 17, 2015
ping ... @nodejs/collaborators @rvagg |
jasnellforce-pushed the doc-addon-improvements branch from bb9b8a9 to 7dfd86fCompare
doc/api/addons.markdown Outdated
jasnell commented Dec 18, 2015
@rvagg ... thank you for the review! Pushed an update with fixes... still need to figure out #4320 (comment) tho... will look at that next |
jasnell commented Dec 18, 2015
@rvagg ... ok, added some language on the deps headers. PTAL |
doc/api/addons.markdown Outdated
There was a problem hiding this comment.
The filenames should have use lowercase v8.
jasnell commented Dec 18, 2015
@ofrobots ... fixed! |
doc/api/addons.markdown Outdated
There was a problem hiding this comment.
require is not a statement
General improvements to the documentation in addons.markdown.
jasnellforce-pushed the doc-addon-improvements branch from 66f9dbe to ed570d5Comparedoc/api/addons.markdown Outdated
There was a problem hiding this comment.
In few other places it is referred as V8.
There was a problem hiding this comment.
sigh lol... I keep missing these. Hopefully this is the last one
jasnell commented Dec 22, 2015
@thefourtheye ... fixed! |
thefourtheye commented Dec 22, 2015
@jasnell I didn't try all the code examples. But except the comment about dynamically linked thingy, everything else LGTM. |
jasnell commented Dec 22, 2015
+1 ... I dropped the "statically" in that one paragraph. |
thefourtheye commented Dec 22, 2015
Actually I would like to understand that paragraph better. So, if you don't mind, let's wait for one more LGTM. If the Addons were to dynamically link to V8, then the V8 has to be compiled and installed as a separate library in the target machine, right? Only then the Addons can load them at runtime. Is that the case here? |
bnoordhuis commented Dec 22, 2015
The node binary exports the public symbols from libv8.a. Add-ons themselves don't load libv8, their references to V8 API functions are resolved by the dynamic linker at run-time to the ones from the node binary. |
jasnell commented Dec 23, 2015
Given the couple of days that have passed and no further comments, I'm going to go ahead and land this. |
General improvements to the documentation in addons.markdown. PR-URL: #4320 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
jasnell commented Dec 23, 2015
Landed in d5863bc |
MylesBorins commented Dec 30, 2015
jasnell commented Dec 30, 2015
I'll handle porting these. |
General improvements to the documentation in addons.markdown. PR-URL: nodejs#4320 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
jasnell commented Jan 29, 2016
@thealphanerd ... will be porting this to LTS early next week |
General improvements to the documentation in addons.markdown. PR-URL: nodejs#4320 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
General improvements to the documentation in addons.markdown. PR-URL: #4320 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
General improvements to the documentation in addons.markdown. PR-URL: #4320 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
General improvements to the documentation in addons.markdown. PR-URL: #4320 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
General improvements to the documentation in addons.markdown. PR-URL: nodejs#4320 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
8 participants
General improvements to the documentation in addons.markdown.
/cc @nodejs/documentation