Skip to content

doc: minor fixes to README.md - #190

Closed
digitalinfinity wants to merge 1 commit into
nodejs:masterfrom
digitalinfinity:readme_fixes
Closed

doc: minor fixes to README.md#190
digitalinfinity wants to merge 1 commit into
nodejs:masterfrom
digitalinfinity:readme_fixes

Conversation

@digitalinfinity

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread README.md Outdated
@@ -5,13 +5,12 @@ Node.js API** also known as **N-API**, providing C++ object model and exception
handling semantics with low overhead. It guarantees backward compatibility with
use with older versions of Node.js that do not have N-API built-in.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It guarantees backward compatibility when used with older versions of Node.js that do not have N-API built-in.

@aruneshchandra aruneshchandra left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How about rearranging the text like this:

This package contains header-only C++ wrapper classes for the ABI-stable Node.js API also known as N-API, providing C++ object model and exception handling semantics with low overhead. It guarantees backward compatibility when used with older versions of Node.js that do not have N-API built-in.

N-API is an API for building native addons. It is independent from the underlying JavaScript runtime (e.g. V8 or ChakraCore) and is maintained as part of Node.js itself. This API will be Application Binary Interface (ABI) stable across versions and flavors of Node.js. It is intended to insulate native addons from changes in the underlying JavaScript engine and allow modules compiled for one version to run on later versions of Node.js without recompilation. N-API guarantees the API and ABI compatibility across different versions of Node.js. So if you switched to a different version of Node.js, you would not need to reinstall or recompile the native addon.

Comment thread README.md Outdated

N-API is an API for building native Addons. It is independent from the underlying
JavaScript runtime (ex V8) and is maintained as part of Node.js itself. This API
JavaScript runtime (e.g. v8) and is maintained as part of Node.js itself. This API

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should be V8 as I believe I saw a change somewhere else to make updates to have it be consistently V8 as opposed to v8.

Comment thread README.md
- **[License](#license)**

## **Current version: 1.0.0**
## **Current version: 1.1.0**

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching this.

@mhdawson mhdawson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM after comment about v8 is addressed.

@mhdawson

Copy link
Copy Markdown
Member

@digitalinfinity just wondering if you are going to update.

@digitalinfinity

Copy link
Copy Markdown
Contributor Author

Ah completely forgot I had this PR open- I'll update today

@digitalinfinity

Copy link
Copy Markdown
Contributor Author

I keep forgetting about this PR- updated it with Arunesh's proposed text

@aruneshchandra aruneshchandra left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanx! LGTM

digitalinfinity added a commit that referenced this pull request Dec 7, 2017
PR-URL: #190
Reviewed-By: Arunesh Chandra <archandr@microsoft.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@digitalinfinity

Copy link
Copy Markdown
Contributor Author

Thanks- landed in 5441b3c

kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
PR-URL: nodejs/node-addon-api#190
Reviewed-By: Arunesh Chandra <archandr@microsoft.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
PR-URL: nodejs/node-addon-api#190
Reviewed-By: Arunesh Chandra <archandr@microsoft.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
PR-URL: nodejs/node-addon-api#190
Reviewed-By: Arunesh Chandra <archandr@microsoft.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
PR-URL: nodejs/node-addon-api#190
Reviewed-By: Arunesh Chandra <archandr@microsoft.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
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