doc: fix JSON generation for aliased methods #4871
Conversation
mscdex
added
doc
|
/cc @nodejs/documentation |
|
Wouldn't it be better to fix it during JSON generation? Because the markdown renders fine and a change there doesn't seem to be something we should wish for, imo. What's your use case? |
|
There is precedence for this type of change: https://nodejs.org/api/https.html#https_server_listen_handle_callback Just a friendly reminder that the whole doc generating process is pretty fragile, so I would +1 this change, as it already exists in the HTTPS docs. |
|
Okay. Could we have a |
|
Hi, https://nodejs.org/api/buffer.html#buffer_buf_readdoublebe_offset_noassert is not deeper and it uses the same style. |
|
Or actually it is. But I still don't see how that affects the argument. |
|
It's bout consistency and readability. Also prettiness and semantics, since a heading without content looks like a bug to me. Even if there were precedence. |
|
@eljefedelrodeodeljefe, you will find this style all over the docs. It is not a bug. It is by design. The only argument against this is resorting the documentation which would cause duplicate documentation because we will have to move As I have done this before, I am sure it is fine now. |
|
Then make the case. -0 |
|
@eljefedelrodeodeljefe I would argue the case is FOR consistency and readability. tbh, this was a mistake on my part in missing this when I reordered all the docs alphabetically, this should've already happened. |
|
@tflanagan that's a good point. How about I move On the doc toolchain being fragile, I totally agree. @eljefedelrodeodeljefe there are code in the toolchain to specifically handle this sort of headings: https://github.com/nodejs/node/blob/master/tools/doc/json.js#L38-L39. So I'd argue that the headings are like this by design. |
Currently assert/assert.ok currently has the following signature:
"signatures": [
{
"params": [
{
"name": "value"
},
{
"name": "message])"
},
{
"name": "assert.ok(value"
},
{
"name": "message",
"optional": true
}
]
}
]
The heading reads
assert(value[, message]), assert.ok(value[, message])
Split them into two sections to make it working.
|
I have just splitted the section into two, so that the alphabetical order is preserved. |
|
This LGTM. Making the headings more regular is great (even if it is to fix the JSON generator, which is pretty neglected!) |
| If `value` is not truthy, an `AssertionError` is thrown with a `message` | ||
| property set equal to the value of the `message` parameter. If the `message` | ||
| parameter is `undefined`, a default error message is assigned. | ||
| An alias of [`assert.ok()`][] . |
There was a problem hiding this comment.
All the other references in this document use the format assert.ok.
There was a problem hiding this comment.
There was a problem hiding this comment.
All I am saying is that, it should be consistent.
|
LGTM now. |
Currently assert/assert.ok currently has the following signature:
"signatures": [
{
"params": [
{
"name": "value"
},
{
"name": "message])"
},
{
"name": "assert.ok(value"
},
{
"name": "message",
"optional": true
}
]
}
]
The heading reads
assert(value[, message]), assert.ok(value[, message])
Split them into two sections to make it working.
PR-URL: nodejs#4871
Reviewed-By: Chris Dickinson <[email protected]>
|
Landed in 27c8b73 |
|
lts-watch label applied |
Currently assert/assert.ok currently has the following signature:
"signatures": [
{
"params": [
{
"name": "value"
},
{
"name": "message])"
},
{
"name": "assert.ok(value"
},
{
"name": "message",
"optional": true
}
]
}
]
The heading reads
assert(value[, message]), assert.ok(value[, message])
Split them into two sections to make it working.
PR-URL: #4871
Reviewed-By: Chris Dickinson <[email protected]>
|
This PR will have to wait for a backlog of doc fixes to land before we can land it in LTS |
|
👍 |
Currently assert/assert.ok currently has the following signature:
"signatures": [
{
"params": [
{
"name": "value"
},
{
"name": "message])"
},
{
"name": "assert.ok(value"
},
{
"name": "message",
"optional": true
}
]
}
]
The heading reads
assert(value[, message]), assert.ok(value[, message])
Split them into two sections to make it working.
PR-URL: #4871
Reviewed-By: Chris Dickinson <[email protected]>
For example assert/assert.ok currently has the following signature:
While the heading reads
Split them into two sections to make it working.