-
Notifications
You must be signed in to change notification settings - Fork 133
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
Update to 1.39.0 [ Jenkins ] #275
Conversation
* update gitignore * update path to electron-main script * upgrade dependencies compatible with node 16 * inversify update changes Contributed on behalf of STMicroelectronics Signed-off-by: Johannes Faltermeier <[email protected]>
Contributed on behalf of STMicroelectronics Signed-off-by: Johannes Faltermeier <[email protected]>
Please note that this does not yet use a bundled backend (eclipse-theia/theia#12412) When I try to create a bundled backend locally, I get the following error during webpack. This is on an ubuntu machine, so I don't know why it tries to package windows certs.
|
I looks like I can reproduce this behaviour with the electron example from the main Theia repository as well by replacing So either this is a general issue with Ubuntu/Linux or with my local environment. |
@jfaltermeier There is some weird issue with the bundler going on, and I'm not sure why it occurs. I've provided a commit over at 12f3d2b that fixes the backend build and runtime issues. |
Thanks! I will update my WIP commit accordingly tomorrow and will look into using the minimized build with electron-builder. |
@jfaltermeier looks good generally. I will review more deeply after you make the update tomorrow. |
* adjust electron-builder config and Dockerfile Co-authored-by: Johannes Faltermeier <[email protected]> Signed-off-by: Johannes Faltermeier <[email protected]>
@marcdumais-work I've updated the PR. |
@marcdumais-work I would like to post the release announcement tomorrow if possible, so if you could apporve today in case you do not find issue, that would be great! |
I've pulled the latest version of the PR branch and am building - will test soon |
There seems to be an issue with
|
I will look into it. Note that this is happening for the build step that tries to adjust some metadata produced by electron-builder after the application was already built. So this only affects the metadata for the updater and a fix should not affect the application itself. |
I see, thanks. Our messages crossed above - have you seen mine? I think you are missing the GitHub token in that final step, which can lead to random failures to download the |
Thanks, I've just pushed a commit that tries that |
@jfaltermeier I wonder, do you know why it's necessary to run |
I try to improve the repo's build scripts in my WIP PR, that I will rebase on this one here after it's merged. |
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, thanks @jfaltermeier
We need to have Jenkins CI pass. i have built and tested locally - seems to work fine. Now we have detachable windows on the Electron app! I do not have access to a Windows machine to test, so there might be some uncertainty there (I tested on linux Ubuntu).
Jenkins CI has passed! |
Thanks @jfaltermeier and @msujew! |
I think it might be required because this is a stash that was initially built on windows, but the signing/upload steps are performed on ubuntu again. |
Makes sense, thanks @jfaltermeier . In my WIP PR, running "yarn install" will be a valid alternative, since it will do much less vs now (similarly to the main Theia repo) |
What it does
Contributed on behalf of STMicroelectronics
How to test
Build and check that everything is still working
Review checklist
Reminder for reviewers