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.

Sign up for GitHub

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

Jump to bottom

Add support for Node 12 #2633

Merged
merged 7 commits into from Apr 26, 2019
Merged

Add support for Node 12 #2633

merged 7 commits into from Apr 26, 2019

Conversation

xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 23, 2019

Fixes #2632

@xzyfer xzyfer self-assigned this Apr 23, 2019
@xzyfer xzyfer added the Node 12 label Apr 23, 2019
@xzyfer
Copy link
Contributor Author

xzyfer commented Apr 23, 2019

Looks like https://github.com/nodejs/nan doesn't yet support Node 12 so we're blocked on compiling Node 12 binaries.

@xzyfer xzyfer mentioned this pull request Apr 23, 2019
Closed
@xzyfer
Copy link
Contributor Author

xzyfer commented Apr 24, 2019

Created a tracking issue nodejs/nan#849

@nschonni
Copy link
Contributor

Also will need appveyor/ci#2921

@j3j5 j3j5 mentioned this pull request Apr 24, 2019
Closed
@xzyfer xzyfer force-pushed the node-12 branch 2 times, most recently from 9d820c1 to b110d0b Compare April 25, 2019 02:29
nschonni
nschonni reviewed Apr 25, 2019
View reviewed changes
appveyor.yml Outdated Show resolved Hide resolved
nschonni
nschonni reviewed Apr 25, 2019
View reviewed changes
.travis.yml Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
@shiftkey shiftkey mentioned this pull request Apr 25, 2019
Merged
@xzyfer
Copy link
Contributor Author

xzyfer commented Apr 26, 2019

Got this green using the workaround from appveyor/ci#2921 (comment). Hopefully this should mean no more waiting for AppVeyor.

@sass sass locked and limited conversation to collaborators Apr 26, 2019
nschonni
nschonni approved these changes Apr 26, 2019
View reviewed changes
Copy link
Contributor

@nschonni nschonni left a comment

Choose a reason for hiding this comment

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

LGTM

@nschonni
Copy link
Contributor

Might want to update the package.json with this, unless you are going to do that separately before tagging

@xzyfer xzyfer merged commit bc78b22 into sass:master Apr 26, 2019
@xzyfer xzyfer deleted the node-12 branch April 26, 2019 10:17
@xzyfer
Copy link
Contributor Author

xzyfer commented Apr 26, 2019

Bumped package.json on master

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nschonni nschonni nschonni approved these changes

Assignees

@xzyfer xzyfer

Labels
Node 12
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

support for node.js 12.x
3 participants
@xzyfer @nschonni @XhmikosR