doc: add builtin module in building.md#17705
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added doc 
Fishrock123 commented Dec 16, 2017
I feel like this documentation belongs in the building docs and not in the API docs. |
Suixinlei commented Dec 16, 2017
Thank you for your advice.Should I close this pr and open the other one? @Fishrock123 |
joyeecheung commented Dec 16, 2017
@Suixinlei You can just move the changes to |
Suixinleiforce-pushed the doc-builtin-module branch 2 times, most recently from 2d48d8f to 478d696CompareSuixinlei changed the title Suixinlei commented Dec 16, 2017
@joyeecheung done. I've updated the pr and the commit. |
BUILDING.md Outdated
There was a problem hiding this comment.
Thank you!
Some tiny fixes:
- that core modules -> those core modules.
- Node -> Node.js.
There was a problem hiding this comment.
@vsemozhetbyt Thanks a lot. I've already update my code.
Suixinleiforce-pushed the doc-builtin-module branch 2 times, most recently from ad10522 to 615f11aCompare
vsemozhetbyt commented Dec 16, 2017
XadillaX left a comment
XadillaX left a comment There was a problem hiding this comment.
I think the commit message shouldn't include Fixes since it's just a documentation changing. Maybe we can use Refs instead.
BUILDING.md Outdated
There was a problem hiding this comment.
The word "those" here looks a bit unnatural since the core modules are not mentioned before this section, and the fact that they are in the lib/ folder is not entirely relevant here since the internal modules are not exposed to users. I would suggest something simpler like:
It is possible to specify one or more JavaScript text files to be bundled in the binary as builtin modules when building Node.js. BUILDING.md Outdated
There was a problem hiding this comment.
We don't really need these many levels here, I think ### is enough.
BUILDING.md Outdated
There was a problem hiding this comment.
Can you add an explanation for this command? Maybe something like To make `/root/myModule.js` available via `require('root/myModule')`:
BUILDING.md Outdated
There was a problem hiding this comment.
Can you add an explanation for this command as well?
joyeecheung commented Dec 18, 2017
BTW this does not fix #12516 which was asking about the bundled native modules (addons), not the JS modules. |
BridgeAR commented Jan 5, 2018
Ping @Suixinlei |
Suixinlei commented Jan 9, 2018
sorry, I wll fix this asap. |
Suixinleiforce-pushed the doc-builtin-module branch from 615f11a to f34ce3bCompare
Suixinleiforce-pushed the doc-builtin-module branch from f34ce3b to 922c53eCompareSuixinlei commented Jan 9, 2018
@joyeecheung thx for your advice, fix done. |
joyeecheung commented Jan 17, 2018
joyeecheung added the author ready 
joyeecheung commented Jan 18, 2018
Landed in f2e62b5, thanks! |
Fixes: #12516 Refs: #2497 PR-URL: #17705 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
addaleax removed the author ready 
Fixes: #12516 Refs: #2497 PR-URL: #17705 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: #12516 Refs: #2497 PR-URL: #17705 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: #12516 Refs: #2497 PR-URL: #17705 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: #12516 Refs: #2497 PR-URL: #17705 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
Fixes: nodejs#12516 Refs: nodejs#2497 PR-URL: nodejs#17705 Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]>
10 participants
Add builtin module doc in
BUILDING.mdand introduce--link-modulesin doc.Fixes: #12516
Refs: #2497
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
doc