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

electron-prebuilt upgraded to latest release #2

Merged
merged 1 commit into from
Apr 7, 2016

Conversation

ppitonak
Copy link
Contributor

@ppitonak ppitonak commented Apr 6, 2016

No description provided.

@twolfson
Copy link
Owner

twolfson commented Apr 6, 2016

What's the reason behind this PR? This is a development dependency so it only affects testing. However, if we want to expand coverage of our testing with respect to Electron, then we should add tests for the past 2 major versions as well (e.g. 0.35.x, 0.36.x)

@twolfson
Copy link
Owner

twolfson commented Apr 6, 2016

As a heads up, this package currently chokes at requiring submodules in the same context as the same window scope.

We have patched it by switching to window.open in https://github.com/twolfson/karma-electron/tree/dev/fix.nested.context but that requires a patch from karma which is being reviewed:

karma-runner/karma#1984

@ppitonak
Copy link
Contributor Author

ppitonak commented Apr 7, 2016

The main reason for this PR is that I consider it a good practice to test with latest stable release. In the case that Electron team supports multiple streams, it makes sense to run tests on all stable releases.

@twolfson
Copy link
Owner

twolfson commented Apr 7, 2016

I don't believe Electron offers multiple streams but we could set something up in Travis CI that does npm install electron-prebuilt@version based on environment variables.

That being said, I don't have time at the moment to take that on so I will land this PR as is =/

Thanks for the quick response

@twolfson twolfson merged commit c8bbd2a into twolfson:master Apr 7, 2016
@twolfson
Copy link
Owner

twolfson commented Apr 7, 2016

This has been merged/released in 3.0.5 (patch release since it doesn't affect non-development usage)

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