pulsar-edit / superstring

Native core components for Pulsar
MIT License
1 stars 5 forks source link

Candidate for new `master` #15

Closed savetheclocktower closed 3 months ago

savetheclocktower commented 3 months ago

This PR proves that the state of #13 is identical to what I've done here: keep all commits, except explicitly revert the parts of #1 that change the source code.

I have more faith that this will merge cleanly into master than #13 — which I made by starting from an earlier commit and cherry-picking everything except for #1.

@mauricioszabo, my reasoning here is that

Once we merge this, we can point Pulsar's package.json to our copy of superstring for the first time — as has been made necessary by the fact that we must explicitly build our own copy of libiconv on macOS.

savetheclocktower commented 3 months ago

Also happy to adopt the CI stuff recently suggested by @DeeDeeG, though I'm happy to back it out again if they think it should be part of a separate PR.

savetheclocktower commented 3 months ago

Failures on Node 18 (on all platforms) are not necessarily expected, but not surprising either.

Failures on macOS on Node 14/16 are acceptable — two specs are failing on an obscure encoding.

Landing this! (Would land this if I had a green button to click.)

DeeDeeG commented 3 months ago

I'm fairly confident the Node 18 failures are from reverting the code we cherry-picked from NAN, unfortunately :/.

But this is fine for now. Progress in current Pulsar is good, doing custom stuff on separate branches for WIP branches of Pulsar is still an option.

EDIT to clarify: I'm referring to the code from #1.