-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
It's only used in a test.
So I ran into these failures during my N-API work and was scratching my head for a good while. Turns out a bunch of tests have always been broken due to a subtle bug introduced into the CLI test 4 years ago (535b524#diff-583d616f4b01bf1fa153e71c0cef7de7R98). For some reason a recent change to some Node LTS versions, or AppVeyor itself has caused the run order of tests to change which surfaced this error. I have applied a patch the v5 branch (4b05387). Re-rebase this PR and your CI troubles should go away. |
Given our chat is it correct that is blocked on mapbox/node-pre-gyp#402? |
Depends if we want to try and support both at the same time |
Yeah we do. With a minimum napi version of 3
…On Fri., 20 Jul. 2018, 6:57 am Nick Schonning, ***@***.***> wrote:
Depends if we want to try and support both at the same time
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#2452 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAjZWODAZcKiucb4R69NE2kgAoTO8RaDks5uIPKigaJpZM4VVtS4>
.
|
This is now released in [email protected] |
Came across https://github.com/JCMais/node-libcurl which has a good node-pre-gyp setup, but they're not doing N-API right now |
55a5f73
to
6fdfdc6
Compare
6b1387c
to
b186df6
Compare
Will this replace |
@codler no, it is just as an alternate to our custom build scripts to pull down pre-built binaries. All native modules in node use node-gyp as the fallback build. |
Emulate the parse behaviour of `@media` query declarations. Fixes sass#2452
Just rebased this, haven't tested it so I'll see what the CI says