-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
72c51b2
to
0622e7b
Compare
6dfa324
to
37289ef
Compare
Four years ago there was a test that mutated `process.cwd` but didn't correctly reset it. The altered global state resulted in future tests having incorrect assetions. I only noticed this because for some reason it manifested in failures in only _some_ Node versions when N-API was being used.
910527c
to
6b1387c
Compare
Is N-API here ready for the prime time? I think have already big changes in, let's not put too many... |
Lots of extensions have moved over. We had issues with node 9 initially but
with node >= 10 ci seems happy.
…On Sat, 9 Nov 2019, 10:29 am Marcin Cieślak, ***@***.***> wrote:
Is N-API here ready for the prime time? I think have already big changes
in, let's not put too many...
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2312?email_source=notifications&email_token=AAENSWFFMI6CA2BTB5NL4BLQSXY4PA5CNFSM4EYTBPS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDTVERA#issuecomment-552030788>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAENSWBS2P73OAXH2UB2SJLQSXY4PANCNFSM4EYTBPSQ>
.
|
Sure the code works now - the question is can we release a new binding file with a new version number when the libuv gets upgraded (it gets upgraded pretty often, but rarely breaks compatibility). I wrap my head around this: https://nodejs.org/api/n-api.html#n_api_implications_of_abi_stability We include |
N-API has async APIs we can use instead of direct libuv |
Yes, we should switch to that. Looks like a non-trivial rewrite to me :( |
Definitely. I think I considered before and ruled it out. I need to
refamiliarise myself with the implementation.
…On Sun, 10 Nov 2019, 12:30 am Marcin Cieślak, ***@***.***> wrote:
Yes, we should switch to that. Looks like a non-trivial rewrite to me :(
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2312?email_source=notifications&email_token=AAENSWGIPCATJ2MCMSE7J4DQS23OPA5CNFSM4EYTBPS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDUGF3I#issuecomment-552100589>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAENSWCYV3YFTNTKG44WXQDQS23OPANCNFSM4EYTBPSQ>
.
|
But that means to me we should drop it if we want to kick 5.0 out of the door quickly. Sorry if I am feeling pushy and not contributing enough... :( |
I appreciate your feedback. Node 8 will is in Jan. I agreed we need to
understand the trade offs of uv's abi, and do our best to limit the impact.
I'll keep digging into it.
…On Sun, 10 Nov 2019, 12:36 am Marcin Cieślak, ***@***.***> wrote:
But that means to me we should drop it if we want to kick 5.0 out of the
door quickly. Sorry if I am feeling pushy and not contributing enough... :(
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#2312?email_source=notifications&email_token=AAENSWHXEGUQQHFS3R4C6ZLQS24FDA5CNFSM4EYTBPS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDUGJXA#issuecomment-552101084>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAENSWG4FO7U4J6BPZKSNN3QS24FDANCNFSM4EYTBPSQ>
.
|
Sure, I'll have a look at it too. |
See #2111
Features
Misc
ES6-ify where it improves readability *BREAKING*process.sass
API (@xzyfer) *BREAKING*engines
to supported Node versions ([v5] Indicate the supported Node.js version in package.json #2316, @realityking) *BREAKING*Object.assign
polyfill ([v5] Drop lodash.assign in favor of the native Object.assign #2318, @realityking)Maybe