Cleanup documentation #335

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

doc/async_worker.md

@@ -81,13 +81,13 @@ This method is used to execute some tasks out of the **event loop** on a libuv
 worker thread. Subclasses must implement this method and the method is run on
 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"

doc/async_worker.md

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

doc/async_worker.md

@@ -140,16 +140,15 @@ operations ends. The given function is called from the main event loop thread.
 identifier for the kind of resource that is being provided for diagnostic
 information exposed by the async_hooks API.

-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

doc/async_worker.md

@@ -160,31 +159,30 @@ information exposed by the async_hooks API.
 - `[in] resource`: Object associated with the asynchronous operation that
 will be passed to possible async_hooks.

-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

doc/async_worker.md

 ```

 - `[in] receiver`: The `this` object passed to the called function.
 - `[in] callback`: The function which will be called when an asynchronous
 operations ends. The given function is called from the main event loop thread.

-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

doc/async_worker.md


 ```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

doc/async_worker.md

@@ -215,15 +212,15 @@ information exposed by the async_hooks API.
 - `[in] resource`: Object associated with the asynchronous operation that
 will be passed to possible async_hooks.

-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

doc/async_worker.md


 ```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

doc/async_worker.md

-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

doc/async_worker.md

-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
 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

doc/string.md

-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

doc/async_worker.md

-`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

doc/async_worker.md

-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

doc/async_worker.md

-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

doc/async_worker.md

+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

doc/async_worker.md

-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

doc/handle_scope.md

 ```

-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

doc/promises.md

 # 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

doc/property_descriptor.md

@@ -1,6 +1,6 @@
 # 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 <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>

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 <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>

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

Cleanup documentation #335

Cleanup documentation #335

Conversation

NickNaso commented Sep 11, 2018 • edited Loading

NickNaso commented Sep 17, 2018

gabrielschulhof left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhdawson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickNaso commented Sep 18, 2018 • edited Loading

NickNaso commented Sep 18, 2018

thefourtheye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhdawson commented Sep 21, 2018

NickNaso commented Sep 11, 2018 •

edited

Loading

NickNaso commented Sep 18, 2018 •

edited

Loading