Skip to content
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: fix bulleted list punctuation in BUILDING.md #34849

Closed
wants to merge 0 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Aug 20, 2020

Remove/add periods as appropriate in bulleted lists in BUILDING.md.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Aug 20, 2020
@jasnell jasnell added the fast-track PRs that do not need to wait for 48 hours to land. label Aug 20, 2020
@jasnell
Copy link
Member

jasnell commented Aug 20, 2020

Shouldn't be any reason for this PR to wait to land once it has appropriate review. Fast-track? Please 👍🏻

@rickyes rickyes added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 20, 2020
BUILDING.md Outdated
@@ -531,7 +531,7 @@ to run it again before invoking `make -j4`.
[Visual Studio 2019](https://visualstudio.microsoft.com/downloads/) or
the "Visual C++ build tools" workload from the
[Build Tools](https://visualstudio.microsoft.com/downloads/#build-tools-for-visual-studio-2019),
with the default optional components.
with the default optional components
* Basic Unix tools required for some tests,
[Git for Windows](https://git-scm.com/download/win) includes Git Bash
and tools which can be included in the global `PATH`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bullet is also a single sentence that ends in a period.

Copy link
Member Author

@Trott Trott Aug 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, everything in the list should be the same structure and also grammatically correct. Unfortunately, this one is neither. It starts with a phrase (like the previous items in the list) and then has a full sentence bolted onto it with a comma-splice.

I'm inclined to leave it for another round of fixing later. This PR is all tiny changes that I think should be uncontroversial. Once I start rewording things, moving stuff around, and adding parentheses or whatever, there may be more bike-shedding (which is fine, but I'd like to keep that separate from the more obvious stuff that can/should land quickly).

Trott added a commit that referenced this pull request Aug 20, 2020
Remove/add periods as appropriate in bulleted lists in BUILDING.md.

PR-URL: #34849
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
@Trott
Copy link
Member Author

Trott commented Aug 20, 2020

Landed in 9fd71a9

@Trott Trott closed this Aug 20, 2020
@Trott Trott deleted the list-punctuation branch August 20, 2020 17:30
@richardlau
Copy link
Member

@Trott As an aside, this PR is showing 0 commits and 0 file changed, probably because it was closed before GitHub could update status. There's a note about it at the end of https://github.com/nodejs/node/blob/master/doc/guides/collaborator-guide.md#technical-howto.

@Trott
Copy link
Member Author

Trott commented Aug 21, 2020

@Trott As an aside, this PR is showing 0 commits and 0 file changed, probably because it was closed before GitHub could update status. There's a note about it at the end of https://github.com/nodejs/node/blob/master/doc/guides/collaborator-guide.md#technical-howto.

Ah, thanks for calling my attention to that! I always push to master before pushing to my branch. Hopefully my muscle-memory won't get in the way of me reversing that process. (Maybe it's time for me to write a script.)

BethGriggs pushed a commit that referenced this pull request Aug 24, 2020
Remove/add periods as appropriate in bulleted lists in BUILDING.md.

PR-URL: #34849
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ricky Zhou <[email protected]>
@danielleadams danielleadams mentioned this pull request Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants