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

make node-addon-api examples context-sensistive #139

Merged
merged 3 commits into from
Jun 10, 2020

Conversation

gabrielschulhof
Copy link
Contributor

This removes global static references from the examples, so that they
might work with worker threads.

This removes global static references from the examples, so that they
might work with worker threads.
@gabrielschulhof
Copy link
Contributor Author

Looks like Travis doesn't yet have a version of Node.js 12.x that ships with N-API 6.

@gengjiawen
Copy link
Member

Looks like Travis doesn't yet have a version of Node.js 12.x that ships with N-API 6.

Maybe limit version like this

"engines": {
"node": ">= 10.6.0"
},
"scripts": {

Also github action looks like not using latest Node.js 12 yet

@gengjiawen
Copy link
Member

gengjiawen commented Jun 3, 2020

I give it more test, should '~10 >=10.20 || >=12.17 ' work.

const semver = require('semver')
const assert = require('assert').strict

const rule = '~10 >=10.20 || >=12.17 '
assert.equal(false, semver.satisfies('10.19.0', rule))
assert.ok(semver.satisfies('10.20.0', rule))
assert.equal(false, semver.satisfies('12.16.0', rule))
assert.ok(semver.satisfies('12.19.0', rule))

@gabrielschulhof
Copy link
Contributor Author

@nodejs/n-api it looks like it's not Travis that has old versions of Node.js but GitHub actions. Thus, with the "engines" change proposed by @gengjiawen the examples are actually only run on Travis so we have some testing there.

@gabrielschulhof
Copy link
Contributor Author

It looks like github by default doesn't grab the latest available version of Node.js if one specifies "10.x" or "12.x", but will grab the latest version if one specifies it exactly (I used "10.21.0" and "12.18.0").

@gabrielschulhof
Copy link
Contributor Author

So, now the examples are run on both Travis and GitHub actions.

@gengjiawen gengjiawen merged commit dc86a66 into nodejs:master Jun 10, 2020
@gabrielschulhof gabrielschulhof deleted the make-context-sensitive branch June 12, 2020 18:48
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