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

tools: fix type sniffing for properties in JSON generation #4884

Closed
wants to merge 1 commit into from
Closed

tools: fix type sniffing for properties in JSON generation #4884

wants to merge 1 commit into from

Conversation

TimothyGu
Copy link
Member

Before:

            {
              "textRaw": "`children` {Array} ",
              "name": "children",
              "desc": "<p>The module objects required by this one.\n\n</p>\n"
            },

(https://nodejs.org/api/modules.json)

After:

            {
              "textRaw": "`children` {Array} ",
              "type": "Array",
              "name": "children",
              "desc": "<p>The module objects required by this one.\n\n</p>\n"
            },

@mscdex mscdex added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. and removed doc Issues and PRs related to the documentations. labels Jan 26, 2016
@mscdex
Copy link
Contributor

mscdex commented Jan 26, 2016

LGTM with a couple of nits: the commit should target tools: instead of doc: and the first line of the commit message needs to be 50 characters or less.

@TimothyGu
Copy link
Member Author

@mscdex, done.

@jasnell
Copy link
Member

jasnell commented Jan 27, 2016

LGTM

@mscdex mscdex changed the title doc: fix type sniffing for properties in JSON generation tools: fix type sniffing for properties in JSON generation Jan 27, 2016
@TimothyGu
Copy link
Member Author

Ping.

@Trott
Copy link
Member

Trott commented Jan 30, 2016

I don't think running CI does anything with tools/doc/json.js but just in case...

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

@Trott
Copy link
Member

Trott commented Jan 30, 2016

Not sure what's up with the ARM hosts in CI right now (stuck in pending, won't start a CI run) but everything else is green.

This probably isn't even anything anyone will ever run on ARM anyway since it's internal tooling.

And CI doesn't even exercise this code, so the CI run is kind of a symbolic gesture more than anything else.

So, will land in another minute or two...

@Trott
Copy link
Member

Trott commented Jan 30, 2016

Landed in 5d8c192b42bd4f9bbdc20fd22bb797663901718c

@Trott Trott closed this Jan 30, 2016
@TimothyGu TimothyGu deleted the type-in-props branch January 30, 2016 18:54
Trott pushed a commit to Trott/io.js that referenced this pull request Jan 30, 2016
PR-URL: nodejs#4884
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@Trott
Copy link
Member

Trott commented Jan 30, 2016

Landed in ab45390

rvagg pushed a commit that referenced this pull request Feb 8, 2016
PR-URL: #4884
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
MylesBorins pushed a commit that referenced this pull request Feb 17, 2016
PR-URL: #4884
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
PR-URL: #4884
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@MylesBorins MylesBorins mentioned this pull request Feb 18, 2016
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
PR-URL: #4884
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4884
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants