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

fix(packager): throw error when electron-prebuilt-compile is not found #2

Merged

Conversation

malept
Copy link
Member

@malept malept commented Dec 4, 2016

Mostly useful for existing Electron projects that want to migrate to Electron Forge.

@MarshallOfSound
Copy link
Member

Good call 👍

Brings up an interesting point though, how hard is it to take an existing project and make electron-forge package it.

Maybe adding an electron-forge import command or something with a better name to automate the process of installing dependencies / removing old ones from the project you want to build?

if (!/[0-9]/.test(packageJSON.devDependencies['electron-prebuilt-compile'][0])) {
global._resolveError = () => console.error('You must depend on an EXACT version of "electron-prebuilt-compile" not a range'.red);
}
} else {
Copy link
Member

Choose a reason for hiding this comment

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

I think this else is at the wrong level (hence the test failure). I think it should be on the nested if statement. And both branches of that nested if statement need to return null

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. That was because I resolved a merge conflict incorrectly. I was wondering why the tests failed.

Copy link
Member

Choose a reason for hiding this comment

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

I thought I had flaky tests already and was about to cry 😆 Quite happy that it caught it 👍

@malept malept force-pushed the add-prebuilt-compile-error-msg branch from c5090e9 to d320d9e Compare December 4, 2016 01:56
@malept
Copy link
Member Author

malept commented Dec 4, 2016

Maybe adding an electron-forge import command or something with a better name to automate the process of installing dependencies / removing old ones from the project you want to build?

👍 Sounds like something that should be an interactive experience (yeoman-style, I guess) by default.

@MarshallOfSound
Copy link
Member

I'm going to merge this as it is a flaw in the test 👍

@MarshallOfSound MarshallOfSound merged commit 2344995 into electron:master Dec 4, 2016
@malept malept deleted the add-prebuilt-compile-error-msg branch December 4, 2016 02:06
dsanders11 pushed a commit that referenced this pull request Jan 14, 2023
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