Cleanup documentation by NickNaso · Pull Request #335 · nodejs/node-addon-api

NickNaso · 2018-09-11T09:21:30Z

Start cleaning up documentation like reported here : nodejs/abi-stable-node#280 (comment)
I need to do another pass to complete the work, but if anyone want start the review it could be great.

Remove comments about things under development Include Napi namespace consistently

NickNaso · 2018-09-17T21:48:14Z

Hi everyone I ended to clean up the documentation removing the unnecessary working progress comments and adding the Napi namespace. Please review this work so we can complete the steps to release the first complete version of documentation for node-addon-api.

gabrielschulhof

There are some more places where we should be namespacing. Maybe this needs a systematic search for "&$96;" with a handy "Napi::" in the clipboard. That way, only the class names need to be typed in by hand.

gabrielschulhof · 2018-09-18T16:04:52Z

 a thread other than that running the main event loop.  As the method is not
 running on the main event loop, it must avoid calling any methods from node-addon-api
-or running any code that might invoke JavaScript.  Instead once this method is
+or running any code that might invoke JavaScript. Instead once this method is


nit: comma between "Instead" and "once"

gabrielschulhof · 2018-09-18T16:07:52Z


-Returns an AsyncWork instance which can later be queued for execution by calling
+Returns a `Napi::AsyncWork` instance which can later be queued for execution by calling
 `Queue`.


nit: `Queue` should be fully namespaced.

gabrielschulhof · 2018-09-18T16:08:13Z


-Returns an AsyncWork instance which can later be queued for execution by calling
+Returns a `Napi::AsyncWork` instance which can later be queued for execution by calling
 `Queue`.


nit: `Queue` should be fully namespaced.

gabrielschulhof · 2018-09-18T16:08:27Z


-Returns an AsyncWork instance which can later be queued for execution by calling
+Returns a `Napi::AsyncWork` instance which can later be queued for execution by calling
 `Queue`.


nit: `Queue` should be fully namespaced.

gabrielschulhof · 2018-09-18T16:08:58Z


 ```cpp
-explicit AsyncWorker(const Object& receiver, const Function& callback,const char* resource_name);
+explicit Napi::AsyncWorker(const Object& receiver, const Function& callback,const char* resource_name);


nit: The types of the formal parameters should probably be fully namespaced.

gabrielschulhof · 2018-09-18T16:09:31Z


-Returns an AsyncWork instance which can later be queued for execution by calling
+Returns a `Napi::AsyncWork` instance which can later be queued for execution by calling
 `Queue`.


nit: `Queue` should be fully namespaced.

gabrielschulhof · 2018-09-18T16:10:20Z


 ```cpp
-explicit AsyncWorker(const Object& receiver, const Function& callback, const char* resource_name, const Object& resource);
+explicit Napi::AsyncWorker(const Object& receiver, const Function& callback, const char* resource_name, const Object& resource);


nit: The types of the formal arguments should probably be namespaced as well.

gabrielschulhof · 2018-09-18T16:11:08Z

-The first step to use the `AsyncWorker` class is to create a new class that inherit
-from it and implement the `Execute` abstract method. Typically input to your
+The first step to use the `Napi::AsyncWorker` class is to create a new class that
+inherits from it and implement the `Execute` abstract method. Typically input to your


nit: full namespacing for `Execute`

gabrielschulhof · 2018-09-18T16:11:59Z

 worker will be saved within class' fields generally passed in through its
 constructor.

 When the `Execute` method completes without errors the `OnOK` function callback


nit: full namespacing for `Execute` and `OnOK`. Same with instances below.

mhdawson

LGTM

mhdawson · 2018-09-18T20:30:00Z

-String::New(napi_env env, const char* value);
-String::New(napi_env env, const char16_t* value);
+Napi::String::New(napi_env env, const std::Napi::string& value);
+Napi::String::New(napi_env env, const std::u16Napi::string& value);


I think these 2 might not be right as I don't expect std::Napi

Ok just fixed!

NickNaso · 2018-09-18T20:36:07Z

@mhdawson I'm working on Napi::AsyncWorker now to fix the problems reported by @gabrielschulhof.

NickNaso · 2018-09-18T21:02:05Z

@mhdawson @gabrielschulhof Ended to fix reported problems

thefourtheye

There were a few grammatical issues, which I ignored as I was not very sure.

thefourtheye · 2018-09-19T10:27:37Z

-`OnError` methods are complete the AsyncWorker instance is destructed.
+Once created, execution is requested by calling `Napi::AsyncWorker::Queue`. When 
+a thread is available for execution the `Napi::AsyncWorker::Execute` method will 
+be invoked.  Once `Napi::AsyncWorker::Execute` complets either 


thefourtheye · 2018-09-19T10:29:07Z

-an error message will cause the `OnError` method to be invoked instead of `OnOK`
-once the `Execute` method completes.
+an error message will cause the `Napi::AsyncWorker::OnError` method to be 
+invoked instead of `OnOK` once the `Napi::AsyncWorker::OnError::Execute` method 


Napi::AsyncWorker::OnOK

Also Napi::AsyncWorker::Execute

thefourtheye · 2018-09-19T10:41:21Z

-on the `Execute` method is done the `OnOk` method is called and the results return
-back to JavaScript invoking the stored callback with its associated environment.
+callback that the `Napi::AsyncWorker` base class will store persistently. When
+the work on the `Execute` method is done the `OnOk` method is called and the


Napi::AsyncWorker::OnOK

thefourtheye · 2018-09-19T10:42:37Z

+only create a new instance and pass to its constructor the callback you want to 
+execute when your asynchronous task ends and other data you need for your 
+computation. Once created the only other action you have to do is to call the 
+`Napi::AsyncWorker::Queue` method that will that will queue the created worker 


redundant "that will"

thefourtheye · 2018-09-19T10:42:49Z

-your asynchronous task ends and other data you need for your computation. Once created the
-only other action you have to do is to call the `Queue` method that will that will
-queue the created worker for execution.
+Using the implementation of a `Napi::AsyncWorker` is straight forward. You need 


You only need to?

thefourtheye · 2018-09-19T10:54:24Z

 ```

-Returns the Napi:Env associated with the HandleScope.
+Returns the `Napi:Env` associated with the `Napi::HandleScope`.


Napi::Env

thefourtheye · 2018-09-19T10:56:59Z

 # Promise

-The Promise class, along with its Promise::Deferred class, implement the ability to create, resolve, and reject Promise objects.
+The `Napi::Promise` class, along with its `Napi::Promise::Deferred` class, implement the ability to create, resolve, and reject `Promise` objects.


The last Promise need not be code-quoted, right?

thefourtheye · 2018-09-19T10:58:41Z

 # Property Descriptor

-An [Object](object.md) can be assigned properites via its [DefineProperty](object.md#defineproperty) and [DefineProperties](object.md#defineproperties) function, which take PropertyDescrptor(s) as their parameters. The PropertyDescriptor can contain either values or functions, which are then assigned to the Object. Note that a single instance of a PropertyDescriptor class can only contain either one value, or at most two functions. PropertyDescriptors can only be created through the class methods [Accessor](#accessor), [Function](#function), or [Value](#value), each of which return a new static instance of a PropertyDescriptor.
+A [`Napi::Object`](object.md) can be assigned properites via its [`DefineProperty`](object.md#defineproperty) and [`DefineProperties`](object.md#defineproperties) function, which take PropertyDescrptor(s) as their parameters. The `Napi::PropertyDescriptor` can contain either values or functions, which are then assigned to the `Napi::Object`. Note that a single instance of a `Napi::PropertyDescriptor` class can only contain either one value, or at most two functions. PropertyDescriptors can only be created through the class methods [`Accessor`](#accessor), [`Function`](#function), or [`Value`](#value), each of which return a new static instance of a `Napi::PropertyDescriptor`.


... (object.md#defineproperties) functions && which take PropertyDescriptor(s)

* Cleaning the documentation. * Remove comments about things under development. * Include Napi namespace consistently. PR-URL: #335 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>

mhdawson · 2018-09-21T13:33:17Z

Landed as fc11c94. Thanks for all of the hard work!

* Cleaning the documentation. * Remove comments about things under development. * Include Napi namespace consistently. PR-URL: nodejs/node-addon-api#335 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>

NickNaso added 3 commits September 7, 2018 21:38

Cleaning the documentation

4bd72c3

Remove comments about things under development Include Napi namespace consistently

Removed work in progress comment

b9955e3

Added some changes to clean documentation

c5c8d5a

NickNaso and others added 2 commits September 18, 2018 00:19

Added missing part

6951c13

squash: fix typo

efbacd8

gabrielschulhof reviewed Sep 18, 2018

View reviewed changes

mhdawson approved these changes Sep 18, 2018

View reviewed changes

mhdawson reviewed Sep 18, 2018

View reviewed changes

Added some missed changes

482df54

Fixed problems on AsyncWorker documentation

fd10e91

mhdawson mentioned this pull request Sep 18, 2018

Next release #347

Closed

6 tasks

thefourtheye reviewed Sep 19, 2018

View reviewed changes

NickNaso mentioned this pull request Sep 19, 2018

Added Call and MakeCallback that accept cargs as agruments #344

Closed

NickNaso added 2 commits September 19, 2018 13:33

Some fix after review by @thefourtheye

5769008

Fixed typo in JavaScript word

eaf1e91

gabrielschulhof approved these changes Sep 20, 2018

View reviewed changes

mhdawson closed this Sep 21, 2018

mhdawson mentioned this pull request Sep 21, 2018

Documentation push on node-add-api nodejs/abi-stable-node#280

Closed

Uh oh!

Conversation

NickNaso commented Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NickNaso commented Sep 17, 2018

Uh oh!

gabrielschulhof left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NickNaso commented Sep 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NickNaso commented Sep 18, 2018

Uh oh!

thefourtheye left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NickNaso commented Sep 11, 2018 •

edited

Loading

NickNaso commented Sep 18, 2018 •

edited

Loading