-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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/api/: fixed 2 little issues in modules.md #27853
doc/api/: fixed 2 little issues in modules.md #27853
Conversation
@@ -342,7 +342,7 @@ If the given path does not exist, `require()` will throw an [`Error`][] with its | |||
<!--type=misc--> | |||
|
|||
It is convenient to organize programs and libraries into self-contained | |||
directories, and then provide a single entry point to that library. | |||
directories, and then provide a single entry point to those directories. |
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 would delete this sentence entirely. The next sentence is sufficient (although consider changing folder
to directory
in that sentence). This sentence mostly just gets in the way.
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.
Hi.
sorry for the late response and thanks for the commit. I agree that the suggested version is better.
What should I do now?
Should I now push the changes to the fork and synchronize this Pull Request with other changes that have landed in master by following instructions given at https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#setting-up-your-local-environment, right?
I'm pretty new to GitHub.
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.
Should I now push the changes to the fork and synchronize this Pull Request with other changes that have landed in master by following instructions given at https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#setting-up-your-local-environment, right?
Yes, I believe so. If you run into a problem, let me know. (We can try to walk you through it, or just do it for you.)
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.
Hi again.
I've tried to do what I described above ("...synchronize this Pull Request with other changes that have landed in master...").
Hope, it worked.
If it didn't work and you can "just do it for you.", please do it )
714e0f6
to
73a3c92
Compare
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.
LGTM
This is a minor clarification in two places in modules.md. PR-URL: nodejs#27853 Reviewed-By: Rich Trott <rtrott@gmail.com>
Landed in db013e1 Thanks for the contribution! 🎉 (If you're interested in other possible contributions to Node.js but don't have a good idea of where to start looking, some ideas are posted at https://www.nodetodo.org/next-steps/.) |
This is a minor clarification in two places in modules.md. PR-URL: #27853 Reviewed-By: Rich Trott <rtrott@gmail.com>
Fixed 2 little issues of API documentation ("Modules" file) as described in issue #27640