-
Notifications
You must be signed in to change notification settings - Fork 171
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.
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
Update C++ version to support Node 16 on Mac #215
Conversation
Travis check fails because |
Not sure how I missed this PR. Apologies! LGTM. I'll investigate Travis separately. |
@dhritzkiv Thanks for merging! Could you also release it? It's going to help with development of downstream libraries such as luma.gl |
I'm been trying to on-and-off on my breaks today, but am running into all of the issues with Travis. It seems that node 16 changed some of the requirements for the compiler, which changed the requirements for which linux distro we should use in CI (we were still on Trusty), which changed how the gl libraries behave, which cause the tests to fail, which keeps us from building prebuilt binaries for release. And that's just the linux tests. I'll try again tomorrow. |
🤯 ok, thanks for the info |
Sometimes OSS maintenance is fun and rewarding… other times, its spent updating dependencies 😄 |
When does the new version publish? |
Sorry everyone, the CI was giving me lots of trouble with tests, and without CI, I couldn't produce a release. This was further compounded by running out of open source credits to use towards CI, so I couldn't test as frequently. This fix is now released in v4.9.2 |
Compiling on Node 16 currently fails with:
I tracked it down to that std::remove_cv_t was added in C++14 https://en.cppreference.com/w/cpp/types/remove_cv while headless-gl compilation forces C++11 on Mac https://github.com/stackgl/headless-gl/blob/master/binding.gyp#L44
After updating C++ version, it compiles well. Running tests, I get a test failure which was already reported as #171, so I suppose it's unrelated.