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

Clean up AsyncProgressWorker documentation #831

Merged
merged 4 commits into from
Nov 9, 2020

Conversation

mastergberry
Copy link
Contributor

The old documentation was not very clear on how to use the API. It was missing javascript references, and was not necessarily following the best coding standards.

These adjustments make it a bit more complete now by providing all three necessary parts, the worker, the hookup code, and the javascript usage of it.

I have also gone ahead and rewritten the AsyncProgressQueueWorker example to show demonstration of the FunctionReference class and how to use multiple callbacks instead of one combined callback.

The old documentation was not very clear on how to use the API. It was missing javascript references, and was not necessarily following the best coding standards.

These adjustments make it a bit more complete now by providing all three necessary parts, the worker, the hookup code, and the javascript usage of it.

I have also gone ahead and rewritten the `AsyncProgressQueueWorker` example to show demonstration of the `FunctionReference` class and how to use multiple callbacks instead of one combined callback.
Copy link
Member

@NickNaso NickNaso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -550,7 +550,7 @@ const onProgressCallback = (num) => {
// ...
};

// Call our native addon with the paramters of a string and a function
// Call our native addon with the paramters of a string three callback functions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it shlould be:
of a string and three callback functions

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why I should not adjust my code late at night :( Fixing it

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mhdawson mhdawson merged commit 1b52c28 into nodejs:master Nov 9, 2020
Superlokkus pushed a commit to Superlokkus/node-addon-api that referenced this pull request Nov 20, 2020
* Clean up AsyncProgressWorker documentation

The old documentation was not very clear on how to use the API. It was missing javascript references, and was not necessarily following the best coding standards.

These adjustments make it a bit more complete now by providing all three necessary parts, the worker, the hookup code, and the javascript usage of it.

I have also gone ahead and rewritten the `AsyncProgressQueueWorker` example to show demonstration of the `FunctionReference` class and how to use multiple callbacks instead of one combined callback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants