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

[v12.x backport] doc: remove em dashes #32146

Closed

Conversation

puzpuzpuz
Copy link
Member

Backports the following PRs to 12.x-staging:

Both of these PRs are necessary to have a successful build on 12.x-staging. See #32131 (comment) and #32131 (comment) to understand the context.

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

They fail on OS X 10.15 (aka "Catalina"), but pass on earlier OS X.

Refs: nodejs#30030
Refs: nodejs/build#2189 (comment)

PR-URL: nodejs#31936
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. v12.x labels Mar 8, 2020
Our documentation uses em dashes inconsistently. They are treated
inconsistently typographically too. (For example, they are sometimes
surrounded by spaces and sometimes not.) They are also often confused
with ordinary hyphens such as in the CHANGELOG, where they are
inadvertently mixed together in a single list. The difference is
not obvious in the raw markdown but is very noticeable when rendered,
appearing to be a typographical error (which it in fact is).

The em dash is never needed. There are always alternatives. Remove em
dashes entirely.

PR-URL: nodejs#32080
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@puzpuzpuz
Copy link
Member Author

puzpuzpuz commented Mar 8, 2020

It looks like Travis CI failed due to timeout in compilation phase. I've already seen such failure here: #32131 (comment)

Both compilation and tests run successfully on my machine.

@puzpuzpuz puzpuzpuz changed the title [v12.x] doc: backport remove em dashes to v12.x [v12.x backport] doc: remove em dashes Mar 8, 2020
@nodejs-github-bot

This comment has been minimized.

@puzpuzpuz
Copy link
Member Author

puzpuzpuz commented Mar 8, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29631/

It seems that there are some flaky tests remaining in v12.x-staging:

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/29637/

@puzpuzpuz
Copy link
Member Author

@vdeturckheim @Qard guys, could one of you review this one? We need it to unblock #32131

Copy link
Member

@vdeturckheim vdeturckheim left a comment

Choose a reason for hiding this comment

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

LGTM

@puzpuzpuz
Copy link
Member Author

Is there something I should do here or this PR is ready to land? I have no experience with backports in core, so that's why I'm asking.

@Trott
Copy link
Member

Trott commented Mar 10, 2020

Is there something I should do here or this PR is ready to land? I have no experience with backports in core, so that's why I'm asking.

Once a 12.x release is being prepared, someone on @nodejs/releasers will start landing these. I don't think there's anything to do here other than wait. Since 12.x is an LTS release, only releasers are supposed to land stuff to the 12.x-staging branch.

@puzpuzpuz
Copy link
Member Author

@Trott thanks for the clarification. Then I'll be waiting for a releaser on this one. Once this one is merged, I'll be unblocked with #32131 and that PR will be able to have a successful CI build.

@codebytere
Copy link
Member

Landed in 5a537ba..ebc47f8

@codebytere codebytere closed this Mar 14, 2020
codebytere pushed a commit that referenced this pull request Mar 14, 2020
They fail on OS X 10.15 (aka "Catalina"), but pass on earlier OS X.

Refs: #30030
Refs: nodejs/build#2189 (comment)

PR-URL: #31936
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>

PR-URL: #32146
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
codebytere pushed a commit that referenced this pull request Mar 14, 2020
Our documentation uses em dashes inconsistently. They are treated
inconsistently typographically too. (For example, they are sometimes
surrounded by spaces and sometimes not.) They are also often confused
with ordinary hyphens such as in the CHANGELOG, where they are
inadvertently mixed together in a single list. The difference is
not obvious in the raw markdown but is very noticeable when rendered,
appearing to be a typographical error (which it in fact is).

The em dash is never needed. There are always alternatives. Remove em
dashes entirely.

PR-URL: #32080
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

PR-URL: #32146
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
@puzpuzpuz puzpuzpuz deleted the remove-dash-backport-v12.x branch March 14, 2020 19:46
@codebytere codebytere mentioned this pull request Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants