-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
make electron runtime deps optional #4873
Conversation
4735232
to
d4d8967
Compare
Signed-off-by: Anton Kosyakov <[email protected]>
It's ready for the review. Guys please have a look we need to fix theia-apps next images. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your changes look good to me, however, I could not try it in a final, bundled application.
thanks @kittaakos, waiting for travis build and merging if no objections |
Sure, if I understood the issue, the theia-apps are broken anyway so that it won't be worse. |
@kittaakos it will allow to fix them, i more worry about people who build electron products that we break them now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
fix #4871
fix #2758