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: remove em dashes #32080

Merged
merged 1 commit into from
Mar 6, 2020
Merged

doc: remove em dashes #32080

merged 1 commit into from
Mar 6, 2020

Conversation

Trott
Copy link
Member

@Trott Trott commented Mar 4, 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.

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 the meta Issues and PRs related to the general management of the project. label Mar 4, 2020
@Trott
Copy link
Member Author

Trott commented Mar 4, 2020

Here's what the mixing of em dashes with hyphens ends up looking like in the CHANGELOG. Notice the difference between the length of the hyphen/dash between the first three entries and the remaining entries.

Screen Shot 2020-03-03 at 9 28 31 PM

CHANGELOG.md Show resolved Hide resolved
@richardlau
Copy link
Member

The first "inconsistency" is mispelled in the commit message.

@Trott Trott force-pushed the de-dash branch 2 times, most recently from eca3997 to e474cb3 Compare March 4, 2020 18:27
@nodejs-github-bot
Copy link
Collaborator

@richardlau richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 4, 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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott
Copy link
Member Author

Trott commented Mar 6, 2020

Landed in b6cd215

@Trott Trott merged commit b6cd215 into nodejs:master Mar 6, 2020
@Trott Trott deleted the de-dash branch March 6, 2020 06:26
@richardlau
Copy link
Member

richardlau commented Mar 8, 2020

@Trott I'd overlooked when reviewing that the regexp in the doc tool is also in v12.x and v10.x so the changes to the CHANGELOG.md in master have caused failures on supported release lines. Would it be possible for you to see if this PR cherry-picks cleanly onto v12.x-staging and v10.x-staging (in which case we can ask @nodejs/backporters to cherry-pick across to staging) or, if they don't, manually backport?

@puzpuzpuz
Copy link
Member

I've created #32146 to backport this PR to v12.x.

puzpuzpuz pushed a commit to puzpuzpuz/node that referenced this pull request 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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott
Copy link
Member Author

Trott commented Mar 8, 2020

Argh, I should have split the CHANGELOG and doc-tool change into its own commit. Thanks, @richardlau and @puzpuzpuz.

Trott added a commit to Trott/io.js that referenced this pull request 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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott
Copy link
Member Author

Trott commented Mar 8, 2020

v10.x backport: #32149

BethGriggs pushed a commit that referenced this pull request Mar 9, 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
Backport-PR-URL: #32149
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 9, 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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Mar 10, 2020
@BethGriggs BethGriggs mentioned this pull request Mar 12, 2020
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 <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: #32146
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
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. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants