-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: add docs for module.paths #14435
Conversation
Thank you! Some suggestions:
|
Or even better, use |
And maybe the "Refs:" line in the commit message need not be prefixed by "- ". |
I will correct it, thx. |
Change the `module.paths` type to `string[]`. According to alphabetical order, Move the `module.paths` after `module.parent`. Refs: nodejs#14371 (comment)
Should I append Refs every commit, or just the first time? |
@atever Fixing follow up commits will be squashed, so the first time suffices. |
[native addons]: addons.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @nodejs/documentation
This diff appears because we did not have a line break in this last line previously. Maybe it is worth to set some rule for this when doc linting is landed.
@vsemozhetbyt got it, thx. |
Remove excess empty lines
@atever I think the line break after the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can remove the [WIP] tag now.
Gained a lot, thanks for your patience. 😊 |
|
||
* {string[]} | ||
|
||
The search paths for the module. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit terse. Perhaps expand the explanation just a bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that a few more words should be added. Maybe also a link to the parts about module resolution.
@atever would you like to submit another PR?
PR-URL: #14435 Refs: #14371 (comment) Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
PR-URL: #14435 Refs: #14371 (comment) Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]>
Refs: #14371 (comment)
Checklist
Affected core subsystem(s)