doc: fix more type case inconsistencies by silverwind · Pull Request #11697 · nodejs/node · GitHub

Checklist

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

Turns out I missed quite a few types in ff13619 because my sed skills weren't up to par. This commit should fix all remaining issues with primitive type cases.

I also made the syntax with multiple types consistent by removing any whitespace between curly braces. If more people prefer {type | type | type} over {type|type|type}, I can also change to that.

Type replacement was done with:

sed -i -E 's#(\{|\|)\s+?(Boolean|Null|Undefined|Number|String|Symbol)\s+?(\||\})#\1\L\2\3#g' *.md

Integer needs to be fixed as well

Integer is not a valid type. Shall we change them all to number?

I think it would make sense to be specific to keep Integer here because it doesn’t really make sense to pass in non-integer numbers here… the upper-casing is a bit weird but I’d be okay with it, as long as we try to be consistent

(also /cc @ameliavoncat whose awesome PRs also touch these parts of the docs and who might want to keep track of this discussion)

Yeah, I guess it's better to keep it as-is. Per our rule discussed in the last PR, everything except the six primitives should be uppercased.

@silverwind in your commit that changed the case in the doctool, Integer was changed to integer. Either way it doesn't seem right to me that number and integer have different cases.

I think I'll move it out of jsPrimitives and into typeMap, possibly linking it to number docs.

Can you drop the spaces before the colons in this file? We don’t do that anywhere else in the docs

done

@TimothyGu d5d4274 fixes the Integer inconsistency in the doctool. I also added the missing undefined type, which results in a few new links in the doc. I still think we should keep it at Integer because we only lowercase the six primitives.

I still think we should keep it at Integer because we only lowercase the six primitives.

But the issue at hand is that an integer is expressed as a number primitive.

I also just looked at a proposal in TypeScript to add integral types, and though not yet accepted, it also uses lower cased type name (int). And AFAICT, there isn't any disagreement in that thread about its capitalization.

@TimothyGu okay, lowercased integer. I guess a argument could be made about it not being a valid constructor. It's really a odd type, but I see its value in documentation.

Landed in 5f32024.

- fix a number of uppercase types - lowercase 'integer' - consistent formatting in crypto PR-URL: #11697 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

- fix a number of uppercase types - lowercase 'integer' - consistent formatting in crypto PR-URL: nodejs#11697 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

Do we want to backport to v6.x?

I'd say it's not strictly necessary, but it could make landing future doc backports easier.

Ideally, these commits should be landed in tandem:

ff13619
9be03a2
172be50
5f32024

@silverwind would you be willing to put together a PR with those commits?

@MylesBorins not right now, I'm busy on other projects. But if you can wait a bit, I can put something together later.

Backport in #13054

- fix a number of uppercase types - lowercase 'integer' - consistent formatting in crypto PR-URL: #11697 Backport-PR-URL: #13054 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

- fix a number of uppercase types - lowercase 'integer' - consistent formatting in crypto PR-URL: nodejs#11697 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

- fix a number of uppercase types - lowercase 'integer' - consistent formatting in crypto PR-URL: nodejs#11697 Backport-PR-URL: nodejs#13054 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

- fix a number of uppercase types - lowercase 'integer' - consistent formatting in crypto PR-URL: #11697 Backport-PR-URL: #13054 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Mar 5, 2017

lpinca approved these changes Mar 6, 2017

View reviewed changes

TimothyGu suggested changes Mar 6, 2017

View reviewed changes

addaleax mentioned this pull request Mar 6, 2017

crypto: add sign/verify support for RSASSA-PSS #11705

Closed

4 tasks

addaleax reviewed Mar 6, 2017

View reviewed changes

silverwind mentioned this pull request Mar 7, 2017

tools: fix lint issue in doctool #11658

Closed

silverwind force-pushed the type-fixes-part-2 branch from d94efe0 to d5d4274 Compare March 7, 2017 17:23

silverwind mentioned this pull request Mar 7, 2017

tools/doc: improvements for types #11167

Closed

3 tasks

TimothyGu approved these changes Mar 7, 2017

View reviewed changes

jasnell approved these changes Mar 7, 2017

View reviewed changes

silverwind force-pushed the type-fixes-part-2 branch 2 times, most recently from 6acf5f0 to 2d3068b Compare March 8, 2017 16:14

doc: fix more type case inconsistencies

8ad63ec

silverwind force-pushed the type-fixes-part-2 branch from 2d3068b to 8ad63ec Compare March 8, 2017 16:17

silverwind closed this Mar 8, 2017

silverwind deleted the type-fixes-part-2 branch March 8, 2017 16:22

silverwind mentioned this pull request Mar 8, 2017

(v7.x-staging backport) doc type fixes #11750

Closed

italoacasas added the dont-land-on-v7.x label Mar 13, 2017

MylesBorins added backported-to-v7.x and removed dont-land-on-v7.x labels Apr 17, 2017

MylesBorins added the backport-requested-v6.x label May 15, 2017

gibfahn added the backported-to-v6.x label May 16, 2017

gibfahn mentioned this pull request May 16, 2017

(v6.x backport) doc types fixes #13054

Closed

MylesBorins mentioned this pull request Jul 18, 2017

v6.11.2 proposal #14356

Merged

doc: fix more type case inconsistencies by silverwind · Pull Request #11697 · nodejs/node · GitHub

Uh oh!

Uh oh!

doc: fix more type case inconsistencies - #11697

doc: fix more type case inconsistencies#11697silverwind wants to merge 1 commit intonodejs:masternodejs/node:masterfrom silverwind:type-fixes-part-2silverwind/node:type-fixes-part-2Copy head branch name to clipboard

Conversation

silverwind commented Mar 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

TimothyGu Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

silverwind Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

addaleax Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

silverwind Mar 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimothyGu Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

silverwind Mar 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

addaleax Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

silverwind Mar 6, 2017

Choose a reason for hiding this comment

Uh oh!

silverwind commented Mar 7, 2017

Uh oh!

TimothyGu commented Mar 7, 2017

Uh oh!

silverwind commented Mar 7, 2017

Uh oh!

silverwind commented Mar 8, 2017

Uh oh!

MylesBorins commented Apr 17, 2017

Uh oh!

silverwind commented Apr 19, 2017

Uh oh!

MylesBorins commented May 8, 2017

Uh oh!

silverwind commented May 9, 2017

Uh oh!

silverwind commented May 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

doc: fix more type case inconsistencies#11697
silverwind wants to merge 1 commit into
nodejs:masterfrom
silverwind:type-fixes-part-2

silverwind commented Mar 5, 2017 •
edited
Loading

silverwind Mar 6, 2017 •
edited
Loading

silverwind Mar 6, 2017 •
edited
Loading