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

Updated require for electron app. #3

Merged
merged 1 commit into from
May 19, 2016
Merged

Conversation

Vj3k0
Copy link
Contributor

@Vj3k0 Vj3k0 commented May 11, 2016

Fixing failure with electron 1.0.0


This change is Reviewable

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @imlucas and @kangas to be potential reviewers

@Vj3k0
Copy link
Contributor Author

Vj3k0 commented May 14, 2016

Really? Hmm, I got electron error saying that it is not valid electron application. When I did this change, everything worked.
Which version of electron did you use?

@mitchhentges
Copy link

Confirmed, also not working for me unless this PR is merged. Seems to be related to Electron 1.0.0+ removing deprecated usage, and app has to be pulled off of require('electron'), now, instead of in it's own require('app').

See the docs here

@jbleuzen
Copy link

@Vj3k0 I was meaning that your fork was working.
I'm using it until it's merged...

@develar
Copy link

develar commented May 18, 2016

@imlucas Please apply this PR ASAP, because your module is recommended by electron-builder/windows-installeer and widely used. Please accept PR.

@mitchhentges
Copy link

mitchhentges commented May 18, 2016

Hey all, I've forked this to support electron@^1.0.0, and published the fork to npm under the name electric-squirrel.

Thanks again to Vj3k0 for the fix!
See the details here

@aaleks
Copy link

aaleks commented May 19, 2016

+1

@develar
Copy link

develar commented May 19, 2016

@imlucas If you don't have time, just add me to contributor (+ npm) (I maintainer of electron-builder, so, unlikely I will harm ;)) I really don't want to reinvent the wheel and promote @mitchhentges fork :)

In the future will be cool to move this project to electron-userland org.

@develar
Copy link

develar commented May 19, 2016

BTW electron-builder has own fork of Squirrel.Windows, so, in the future, electron-userland/electron-builder#409 may be implemented, and such module will be not required (yeah, so easy to create os x app, and so many troubles under windows...).

@imlucas
Copy link
Contributor

imlucas commented May 19, 2016

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@imlucas imlucas merged commit a84d504 into mongodb-js:master May 19, 2016
@imlucas
Copy link
Contributor

imlucas commented May 19, 2016

published in electron-squirrel-startup@0.2.0

@develar
Copy link

develar commented May 19, 2016

@imlucas Thanks! Maybe it is time to publish it as 1.0.0? Because ^0.1.0 will not cover 0.2. I think there are no planned breaking changes and, so, 1.0.0 can be used.

@imlucas
Copy link
Contributor

imlucas commented May 19, 2016

@develar agreed. published electron-squirrel-startup@1.0.0

@imlucas
Copy link
Contributor

imlucas commented May 19, 2016

@develar added as a contributor

@Vj3k0
Copy link
Contributor Author

Vj3k0 commented May 19, 2016

Thanks a lot! Cheers

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.

7 participants