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.

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

[deps] Use Own Prebuilt Spellchecker #724

Merged
merged 1 commit into from
Dec 19, 2019

Conversation

nsakaimbo
Copy link
Contributor

@nsakaimbo nsakaimbo commented Dec 19, 2019

In order to pre-empt the update of Calypso to node v12, we need to point electron-spellchecker at our own prebuilt fork to ensure prebuilds are fetched for the current version of Electron and not the environment's version of node (as set from Calypso's .nvmrc).

If we upgrade Calypso to latest, the app as-is will fail to build as by default prebuild-install will fetch binaries against the host node environment and not Electron. While the Electron upgrade is pending, I'm forcing prebuild-install to fetch upstream prebuilds for wp-desktop's current version of Electron here, here and here.

prebuild-install WARN install No prebuilt binaries found 
(target=12.13.1 runtime=node arch=x64 platform=darwin)
// ...
// ...
// ...
// BUILD FAILURE

Worth noting: This is yet another reason we shouldn't be using prebuilds in the first place.

In order to pre-empty update of Calypso to Node v12, we need to point electron-spellchecker at our own prebuilt fork which ensures prebuilds are fetched for the current version of Electron and not the environment's version of Node.
Copy link

@belcherj belcherj left a comment

Choose a reason for hiding this comment

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

Ship it.

@nsakaimbo nsakaimbo merged commit 2ad3b5e into develop Dec 19, 2019
@nsakaimbo nsakaimbo deleted the fix/use-own-prebuilt-spellchecker branch December 19, 2019 16:44
@nsakaimbo nsakaimbo mentioned this pull request Jan 2, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants