search

discord / erlpack

High Performance Erlang Term Format Packer
MIT License
217 stars 64 forks source link

feat: Compatibility for Node 16 #41

Closed Hazmi35 closed 2 years ago

Hazmi35 commented 2 years ago

Nodejs 16 uses V8 engine version 9.0, that requires c++14 features, so we need to remove "-std=c++11" from binding.gyp. Idk why "-std=c++11" is added in f3c64a0d54eb603c0b7479036716b1b0f1d089dd

REFS: https://github.com/nodejs/node-gyp/issues/2387#issuecomment-831253742 https://github.com/nodejs/node/issues/38367

Hazmi35 commented 2 years ago

Ah, my bad. I didn't see #40, should I close this PR?

ExperiBass commented 2 years ago

I'll just merge with mines if I can

Hazmi35 commented 2 years ago

I'll just merge with mines if I can

Okay

Hazmi35 commented 2 years ago

Reopening this PR because #40 is closed

Hazmi35 commented 2 years ago

I forgot to delete .node-version file. I used it for fnm to test with Node v16.

jhgg commented 2 years ago

Can you delete the package lock?

Hazmi35 commented 2 years ago

Can you delete the package lock?

Hi sorry, for the late response, I just deleted the lockfile @jhgg

Zoddo commented 2 years ago

While you are at it, merging #42 would be useful too :)

jhgg commented 2 years ago

@adill - when you get a chance, can you double check that this looks sensible?

Hazmi35 commented 2 years ago

Edited the first message for an explanation. Note that this has not been tested in Mac OS, but it works on Node.js v8 to v16 on Arch Linux, Node.js v8 to v16 on Windows (build 19044.1165, msvs 2019), and Node.js v14-v16 on Alpine Linux (docker)

fredkilbourn commented 2 years ago

@jhgg @adill Hi guys, its been a week just bumping to see if you have a minute to review/merge this yet. Thanks!

Milo123459 commented 2 years ago

Yea, eta would be good. Other pr is fucked.

satoufuyuki commented 2 years ago

any progress?

fredkilbourn commented 2 years ago

@jhgg @adill weekly bump, apologies

Hazmi35 commented 2 years ago

Hi, sorry for the bump, any progress on PR Review? Just a friendly reminder that Node 16 will be an Active LTS next month.

fredkilbourn commented 2 years ago

Sorry again, but bump

fredkilbourn commented 2 years ago

Another 2 weeks bump, is it possible to get this merged? @jhgg @adill

msciotti commented 2 years ago

Got final signoff from Jake and Andy. Thanks for this!