tools,doc: lint for additional strings in docs #17492

Trott · 2017-12-06T14:17:17Z

Check for uses of Github (rather than GitHub) and Node.JS (rather
than Node.js) in markdown files.

Checklist

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

Affected core subsystem(s)

tools, doc

cjihrig

LGTM, but additional is spelled wrong in the commit message.

Use `GitHub` rather than `Github` and `Node.js` rather than `Node.JS`.

Check for uses of `Github` (rather than `GitHub`) and `Node.JS` (rather than `Node.js`) in markdown files.

Trott · 2017-12-06T14:31:40Z

Fixed commit message (and PR title) typo. (Thanks, @cjihrig!) Rebased against master while I was at it.

maclover7 · 2017-12-06T14:34:14Z

tools/remark-preset-lint-node/index.js

@@ -42,8 +42,10 @@ module.exports.plugins = [
  [
    require('remark-lint-prohibited-strings'),
    [
-      { no: 'v8', yes: 'V8' },
-      { no: 'Javascript', yes: 'JavaScript' }
+      { no: 'Github', yes: 'GitHub' },


Isn't tools/remark-preset-lint-node/index.js technically provided by watilde/remark-preset-lint-node, and shouldn't be edited downstream? Ref #17441

It's a small thing, but I'd prefer that the version downstream get synchronized with this version (after this lands) rather than landing downstream first. If it lands downstream first, there's a race condition where new errors can creep in here before the updated version lands here. Not too big a deal for us as we can fix the problematic text, but potentially irksome for anyone installing the preset locally and suddenly getting errors on the Node.js master branch.

The problem with the approach I suggest above is that some info in the package.json will be wrong for a period of time. We're not running the version specified in the package.json. We're running a modified version and we need to update it again when this gets published upstream.

Anyway, I don't feel too strongly about this and can be persuaded that this should wait for downstream rather than the other way around if people feel strongly about it. Related: We should probably move the repo to the nodejs org and allow a few more folks to publish it, assuming @watilde has no problem with that.

It's a small thing, but I'd prefer that the version downstream get synchronized with this version (after this lands) rather than landing downstream first.

Ok, fine by me.

My other point (and why I put a link to my PR) was that this (very small) module should live within the nodejs/node repo, so that we don't have to worry about upstreaming/downstreaming changes and publishing versions to npm.

maclover7

LGTM besides for question about remark-preset-lint-node

Trott · 2017-12-06T17:25:45Z

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

Trott · 2017-12-07T04:12:38Z

Raspberry Pi failures are build issues and unrelated. CI is good.

Use `GitHub` rather than `Github` and `Node.js` rather than `Node.JS`. PR-URL: nodejs#17492 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Check for uses of `Github` (rather than `GitHub`) and `Node.JS` (rather than `Node.js`) in markdown files. PR-URL: nodejs#17492 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Trott · 2017-12-08T21:28:38Z

Landed in 3ad01e7 and 762ed33.

Trott · 2017-12-08T21:43:06Z

Here's a PR to downstream to sync: https://github.com/watilde/remark-preset-lint-node/pull/3

Use `GitHub` rather than `Github` and `Node.js` rather than `Node.JS`. PR-URL: #17492 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Check for uses of `Github` (rather than `GitHub`) and `Node.JS` (rather than `Node.js`) in markdown files. PR-URL: #17492 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Use `GitHub` rather than `Github` and `Node.js` rather than `Node.JS`. PR-URL: #17492 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Check for uses of `Github` (rather than `GitHub`) and `Node.JS` (rather than `Node.js`) in markdown files. PR-URL: #17492 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jon Moss <me@jonathanmoss.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Trott added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Dec 6, 2017

nodejs-github-bot added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Dec 6, 2017

cjihrig approved these changes Dec 6, 2017

View reviewed changes

apapirovski approved these changes Dec 6, 2017

View reviewed changes

Trott changed the title ~~tools,doc: lint for additinoal strings in docs~~ tools,doc: lint for additional strings in docs Dec 6, 2017

Trott added 2 commits December 6, 2017 06:30

doc: use correct and consistent typography for products

ab3535d

Use `GitHub` rather than `Github` and `Node.js` rather than `Node.JS`.

tools: lint for additional strings in docs

68da4b0

Check for uses of `Github` (rather than `GitHub`) and `Node.JS` (rather than `Node.js`) in markdown files.

Trott force-pushed the moar-strings branch from 5405e80 to 68da4b0 Compare December 6, 2017 14:31

maclover7 reviewed Dec 6, 2017

View reviewed changes

maclover7 approved these changes Dec 6, 2017

View reviewed changes

jasnell approved these changes Dec 6, 2017

View reviewed changes

danbev approved these changes Dec 7, 2017

View reviewed changes

lpinca approved these changes Dec 7, 2017

View reviewed changes

Trott added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Dec 7, 2017

MylesBorins force-pushed the master branch from b7405ab to 7f086dd Compare December 8, 2017 16:38

Trott closed this Dec 8, 2017

MylesBorins mentioned this pull request Dec 12, 2017

v9.3.0 proposal #17631

Merged

gibfahn added baking-for-lts PRs that need to wait before landing in a LTS release. lts-watch-v6.x and removed baking-for-lts PRs that need to wait before landing in a LTS release. labels Dec 20, 2017

gibfahn added dont-land-on-v6.x and removed lts-watch-v6.x labels Apr 24, 2018

Trott deleted the moar-strings branch January 13, 2022 22:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools,doc: lint for additional strings in docs #17492

tools,doc: lint for additional strings in docs #17492

Trott commented Dec 6, 2017

cjihrig left a comment

Trott commented Dec 6, 2017

maclover7 Dec 6, 2017

Trott Dec 6, 2017 •

edited

Loading

maclover7 Dec 6, 2017

maclover7 left a comment

Trott commented Dec 6, 2017 •

edited

Loading

Trott commented Dec 7, 2017

Trott commented Dec 8, 2017

Trott commented Dec 8, 2017

tools,doc: lint for additional strings in docs #17492

tools,doc: lint for additional strings in docs #17492

Conversation

Trott commented Dec 6, 2017

Checklist

Affected core subsystem(s)

cjihrig left a comment

Choose a reason for hiding this comment

Trott commented Dec 6, 2017

maclover7 Dec 6, 2017

Choose a reason for hiding this comment

Trott Dec 6, 2017 • edited Loading

Choose a reason for hiding this comment

maclover7 Dec 6, 2017

Choose a reason for hiding this comment

maclover7 left a comment

Choose a reason for hiding this comment

Trott commented Dec 6, 2017 • edited Loading

Trott commented Dec 7, 2017

Trott commented Dec 8, 2017

Trott commented Dec 8, 2017

Trott Dec 6, 2017 •

edited

Loading

Trott commented Dec 6, 2017 •

edited

Loading