DeeDeeG
commented
3 months ago I can at least say @mauricioszabo appears to be fairly comfortable working with various dependencies (such as superstring) as git repo URL dependencies, off of specific "working (WIP) branches" of repositories as needed. This may be the "status quo" you were alluding to.
As long as master of superstring remains intended for current Pulsar master, and we PR changes intended for current Pulsar master promptly (or manually test such things before merging here?) then that should cover it.
Otherwise, I agree, efforts to have this done in CI would be worthwhile. We have a thing called action-pulsar-dependency (https://github.com/pulsar-edit/action-pulsar-dependency), unclear if just throwing a uses: pulsar-edit/action-pulsar-dependency@v3.2 into a workflow would take care of our testing needs? See this usage example for action-pulsar-dependency, it's being use already in our github package fork... https://github.com/pulsar-edit/github/blob/5c1590c4537b601d0743bd491c8ac58d5c823856/.github/workflows/ci.yml#L29
Have you checked for existing feature requests?
Summary
This recent saga with
libiconvon macOS illustrated that, up until now, we'd been building Pulsar with the version ofsuperstringthat the Atom folks most recently published to NPM — not the version from our fork against which we've applied a number of commits.When we pointed Pulsar at the head of our fork's
masterbranch, a number of issues were revealed, including very consistent crashes of Pulsar's renderer process during editor and package tests. When we assembled an alternative branch — one consisting of all our changes minus anything that affected the coresuperstringsource code — and built Pulsar against that, all our problems went away.So some of these commits caused breakages in Pulsar. We didn't know at the time, but we were bound to discover it eventually. This strongly suggests that it's not enough for us to run
superstring’s own tests to prove that it works; we should also be running some smoke tests against Pulsar to prove that a proposed change will not regress Pulsar's behavior.What benefits does this feature provide?
The immediate benefit is that it reveals much sooner in the process whether a change to
superstringwill cause a regression in Pulsar.The medium-term benefit is much larger: at some point we'll need to have a version of
superstringthat can run in the most recent version of Electron. That requires a migration to N-API (see #10). At that point it's not just about regressions! It's about threading the needle of being able to support both Electron 12 and Electron ~30 (and climbing) with one version ofsuperstring— or, if that's not possible, maintaining two different versions at once.Either way, this work will necessitate a second set of Pulsar smoke tests in CI: one that tests against an experimental version of Pulsar built against a recent version of Electron.
Any alternatives?
I don't really see a way around this. The status quo is an alternative — one in which we don't find out how a given change affects Pulsar until later in the process — but that feels self-evidently worse.
Other examples:
No response