Skip to content

feat: Compatibility for Node 16 - #41

Merged
msciotti merged 3 commits into
discord:masterfrom
hzmifork:feat/nodejs-v16
Oct 15, 2021
Merged

feat: Compatibility for Node 16#41
msciotti merged 3 commits into
discord:masterfrom
hzmifork:feat/nodejs-v16

Conversation

@Hazmi35

@Hazmi35 Hazmi35 commented Aug 10, 2021

Copy link
Copy Markdown
Contributor

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 f3c64a0

REFS:
nodejs/node-gyp#2387 (comment)
nodejs/node#38367

@Hazmi35

Hazmi35 commented Aug 10, 2021

Copy link
Copy Markdown
Contributor Author

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

@0xf0xx0

0xf0xx0 commented Aug 12, 2021

Copy link
Copy Markdown

I'll just merge with mines if I can

@Hazmi35

Hazmi35 commented Aug 12, 2021

Copy link
Copy Markdown
Contributor Author

I'll just merge with mines if I can

Okay

@Hazmi35 Hazmi35 closed this Aug 12, 2021
@Hazmi35 Hazmi35 reopened this Aug 13, 2021
@Hazmi35

Hazmi35 commented Aug 13, 2021

Copy link
Copy Markdown
Contributor Author

Reopening this PR because #40 is closed

@Hazmi35

Hazmi35 commented Aug 15, 2021

Copy link
Copy Markdown
Contributor Author

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

@jhgg

jhgg commented Aug 15, 2021

Copy link
Copy Markdown
Contributor

Can you delete the package lock?

@Hazmi35

Hazmi35 commented Aug 16, 2021

Copy link
Copy Markdown
Contributor Author

Can you delete the package lock?

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

@Zoddo

Zoddo commented Aug 16, 2021

Copy link
Copy Markdown

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

@jhgg

jhgg commented Aug 16, 2021

Copy link
Copy Markdown
Contributor

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

@Hazmi35

Hazmi35 commented Aug 16, 2021

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown

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

@Milo123459

Copy link
Copy Markdown

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

@satoufuyuki

Copy link
Copy Markdown

any progress?

@fredkilbourn

Copy link
Copy Markdown

@jhgg @adill weekly bump, apologies

@Hazmi35

Hazmi35 commented Sep 23, 2021

Copy link
Copy Markdown
Contributor Author

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

mzrtamp pushed a commit to stegripe/rawon that referenced this pull request Sep 24, 2021
Still no ETA until hzmi's fork merged, use hzmifork or don't add this [discord/erlpack#41](discord/erlpack#41)

Co-authored-by: Zen <45705890+KurokuTetsuya@users.noreply.github.com>
chakany added a commit to chakany/erlpack that referenced this pull request Sep 24, 2021
@fredkilbourn

Copy link
Copy Markdown

Sorry again, but bump

@fredkilbourn

Copy link
Copy Markdown

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

@msciotti

Copy link
Copy Markdown
Contributor

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

@msciotti
msciotti merged commit f7d730d into discord:master Oct 15, 2021
@SuperchupuDev SuperchupuDev mentioned this pull request Oct 19, 2021
@Hazmi35
Hazmi35 deleted the feat/nodejs-v16 branch January 19, 2022 15:20
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.

8 participants