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: fix rebuild:electron command #8454

Merged
merged 1 commit into from
Sep 1, 2020
Merged

Conversation

vince-fugnitto
Copy link
Member

What it does

The following commit fixes the rebuild:electron command to include drivelist as part of the list of native modules to rebuild for the electron target. The pull-request fixes an issue where the electron app fails to start.

How to test

  1. perform yarn start:electron.
  2. the application should successfully start again.

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

the following commit fixes the `rebuild:electron` command to include
`drivelist` as part of the list of native modules to rebuild for the
`electron` target.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto vince-fugnitto added bug bugs found in the application electron issues related to the electron target labels Sep 1, 2020
@vince-fugnitto vince-fugnitto self-assigned this Sep 1, 2020
Copy link
Contributor

@marcdumais-work marcdumais-work left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Vince!

Tested on Gitpod.

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

Works for me too.

@vince-fugnitto vince-fugnitto merged commit d760553 into master Sep 1, 2020
@vince-fugnitto vince-fugnitto deleted the vf/electron-rebuild branch September 1, 2020 17:39
@kittaakos
Copy link
Contributor

The pull-request fixes an issue where the electron app fails to start.

Do you have a link to the error or the bug report? I wonder why it became suddenly mandatory and why wasn't is a problem in the last 2-3 years? Or was it related to dependency updates? Thanks!

@vince-fugnitto
Copy link
Member Author

The pull-request fixes an issue where the electron app fails to start.

Do you have a link to the error or the bug report? I wonder why it became suddenly mandatory and why wasn't is a problem in the last 2-3 years? Or was it related to dependency updates? Thanks!

Yes, it was related to the prior commit for the yarn upgrade: e198c7c
Sorry, I should have opened an issue first then fixed it (we experienced the issue locally and internally after the commit).

@kittaakos
Copy link
Contributor

Yes, it was related to the prior commit for the yarn upgrade: e198c7c
Sorry, I should have opened an issue first then fixed it (we experienced the issue locally and internally after the commit).

No worries, and thank you!

@christlee1989
Copy link

@vince-fugnitto Why i still have error
image
image

@kittaakos
Copy link
Contributor

You have to run yarn rebuild:electron. Did you do that?

@christlee1989
Copy link

You have to run yarn rebuild:electron. Did you do that?

image

@kittaakos
Copy link
Contributor

Hmm, interesting; I never run a Theia build after the electron rebuild but the other way around. Did this work for you earlier? Do you have a public repo I can try?

@christlee1989
Copy link

Hmm, interesting; I never run a Theia build after the electron rebuild but the other way around. Did this work for you earlier? Do you have a public repo I can try?

I used https://github.com/eclipse-theia/theia-example , changed all dependencies version to 'next' , then run yarn, then yarn start

@kittaakos
Copy link
Contributor

I used https://github.com/eclipse-theia/theia-example , changed all dependencies version to 'next' , then run yarn, then yarn start

I did not know this repo exists. Do not use it. Use these steps instead: https://github.com/eclipse-theia/theia/blob/master/doc/Developing.md#quick-start

@christlee1989
Copy link

I used https://github.com/eclipse-theia/theia-example , changed all dependencies version to 'next' , then run yarn, then yarn start

I did not know this repo exists. Do not use it. Use these steps instead: https://github.com/eclipse-theia/theia/blob/master/doc/Developing.md#quick-start

ok, ill try it , thx.

@kittaakos
Copy link
Contributor

ok, ill try it , thx.

👍 Let us know if you're still having issues.

@christlee1989
Copy link

ok, ill try it , thx.

👍 Let us know if you're still having issues.

no error now, but I want to know why theia-example has issue. I want to build my own IDE , could you tell me how to start ? is there any useable & complete example?

@christlee1989
Copy link

The goal is to eventually generate a desktop version IDE

@kittaakos
Copy link
Contributor

but I want to know why theia-example has issue.

You should open an issue there and not comment on a closed PR.

I want to build my own IDE , could you tell me how to start

@christlee1989
Copy link

ok thx

@marcdumais-work
Copy link
Contributor

marcdumais-work commented Sep 4, 2020

no error now, but I want to know why theia-example has issue.

@christlee1989 I think theia-example is a valid example, just not for what you are trying to achieve: That repo has an Electron-only example app and from it we produce a signed, packaged application, for multiple platforms.

Please follow the advise above and inspire yourself from a more generic example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application electron issues related to the electron target
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants