-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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
unneded "." in pull-requests.md file #45502
Comments
It's unclear to me if all items should end with a colon, or if no items should end with a colon. I agree that the lack of consistency is odd though. Refs: https://learn.microsoft.com/en-us/style-guide/scannable-content/lists#punctuation /cc @nodejs/documentation @Trott |
I found a particular article mentioning this:
|
In this case, I would propose one of the following:
Most pull requests in Node.js include changes to the C++ code
in the `src` directory, the JavaScript code in the `lib` directory,
documentation in the `doc/api` directory, and/or test in the `test` directory.
Pull requests in Node.js typically involve changes to
one or more of a few places in the code base.
* C/C++ code contained in the `src` directory
* JavaScript code contained in the `lib` directory
* Documentation in `doc/api`
* Tests within the `test` directory |
I did option 3 in #45519. |
Fixes: nodejs#45502 PR-URL: nodejs#45519 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Version
1af63a9
Platform
github
Subsystem
node/doc/contributing/pull-requests.md
What steps will reproduce the bug?
Intro:
Open the main branch, commit 1af63a9 of nodejs/node
File pull-requests.md
Bug:
There is an unneeded "." in the end of the "tests within the test directory." line.
How often does it reproduce? Is there a required condition?
No response
What is the expected behavior?
No response
What do you see instead?
Additional information
I suggest this bug is marked with "good first time issue" so a new joiner could fix it.
The text was updated successfully, but these errors were encountered: