Update number.md #436
Update number.md #436
Conversation
The docs are apparently wrong when comparing to the string ones. Hope it is correct. If any other changes are required, please tell me and I'll be more than happy to change.
|
Sorry for the many commits... As github allows you to squash when merging I didn't squash myself. I can if it is preferred, though. |
doc/number.md
Outdated
| - `[in] env`: The `napi_env` environment in which to construct the `Napi::Nuber` object. | ||
| - `[in] value`: The `napi_value` which is a handle for a JavaScript `Number`. | ||
| - `[in] env`: The `napi_env` environment in which to construct the `Napi::Number` object. | ||
| - `[in] value`: The primitive to wrap. |
There was a problem hiding this comment.
IMO this explanation is too brief. How about "The JavaScript value holding a number."?
doc/number.md
Outdated
| - `[in] env`: The `napi_env` environment in which to construct the `Napi::Nuber` object. | ||
| - `[in] value`: The `napi_value` which is a handle for a JavaScript `Number`. | ||
| - `[in] env`: The `napi_env` environment in which to construct the `Napi::Number` object. | ||
| - `[in] value`: The C++ primitive from which to instantiate the `Napi::Value`. `value` must be a `c++ double`. |
There was a problem hiding this comment.
IMO the explanation here should be "The value from which to instantiate the `Napi::Number`".
There was a problem hiding this comment.
The fact that the value is a double should be obvious from the prototype.
There was a problem hiding this comment.
I was just keeping in sync with the String one. Ensuring it's similar.
There was a problem hiding this comment.
Thanks for the feedback! Hope it's better now.
doc/number.md
Outdated
| - `[in] env`: The `napi_env` environment in which to construct the `Napi::Nuber` object. | ||
| - `[in] value`: The `napi_value` which is a handle for a JavaScript `Number`. | ||
| - `[in] env`: The `napi_env` environment in which to construct the `Napi::Number` object. | ||
| - `[in] value`: The C++ primitive from which to instantiate the `Napi::Value`. |
There was a problem hiding this comment.
"... from which to instantiate the `Napi::Number`", right, because the return value is Napi::Number?
There was a problem hiding this comment.
In the string page it says Napi::Value, but I changed it. Maybe we need to edit the string docs as well?
There was a problem hiding this comment.
@heynemann we may indeed need to edit the string docs as well.
There was a problem hiding this comment.
However, it's OK if we do that in a second PR.
There was a problem hiding this comment.
Any additional feedback for this PR?
Update docs to be consistent with how thing are described in other sections. PR-URL: #436 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
|
@heynemann, thanks for the update landed as: 7b87e0b |
Update docs to be consistent with how thing are described in other sections. PR-URL: nodejs/node-addon-api#436 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Update docs to be consistent with how thing are described in other sections. PR-URL: nodejs/node-addon-api#436 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Update docs to be consistent with how thing are described in other sections. PR-URL: nodejs/node-addon-api#436 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Update docs to be consistent with how thing are described in other sections. PR-URL: nodejs/node-addon-api#436 Reviewed-By: Gabriel Schulhof <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
The docs are apparently wrong when comparing to the string ones. Hope it is correct. If any other changes are required, please tell me and I'll be more than happy to change.