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.

Sign up for GitHub

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

Jump to bottom

Create Browser Application and Dockerfile #75 #252

Merged
merged 4 commits into from
Mar 8, 2023
Merged

Create Browser Application and Dockerfile #75 #252

merged 4 commits into from
Mar 8, 2023

Conversation

jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Mar 2, 2023 •
edited
Loading

What it does

  • create a dedicated browser application
  • adapt product so that frontend code is useable from browser app as well
  • move electron:rebuild to laters stage to avoid conflicts between both applications
  • Create Dockerfile with Github Workflow and adapt Readme

How to test

  • Check whether electron based app is still working
  • Test browser application with yarn browser start
  • Test Dockerfile and image with instructions from Readme

Review checklist

Reminder for reviewers

@jfaltermeier
Create Browser Application and Dockerfile #75
1ae7c4a 
* create a dedicated browser application
* adapt product so that frontend code is useable from browser app as
well
* move electron:rebuild to laters stage to avoid conflicts between both
applications
* Create Dockerfile with Github Workflow and adapt Readme

Signed-off-by: Johannes Faltermeier <jfaltermeier@eclipsesource.com>
AlexandraBuzila
AlexandraBuzila reviewed Mar 2, 2023
View reviewed changes
Copy link
Contributor

@AlexandraBuzila AlexandraBuzila left a comment

Choose a reason for hiding this comment

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

Changes work great for me and I was able to start both applications.

I noticed however that there's a mismatch between the plugins of the electron and browser app. In particular the 'vscode.git' extension stood out to me, as I recall it might be broken in Theia (at least for the electron version).

applications/browser/package.json Outdated Show resolved Hide resolved
README.md
@@ -95,3 +95,19 @@ yarn electron test

The features in Eclipse Theia Blueprint are based on Theia and the included extensions/plugins. For bugs in Theia please consider opening an issue in the [Theia project on Github](https://github.com/eclipse-theia/theia/issues/new/choose).
Copy link
Contributor

Choose a reason for hiding this comment

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

The browser app should be mentioned in the Repository Structure section above.
Also, the Package the Application section should be probably renamed to Package the Electron Application.
Do we need a section mentioning how to start the browser app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the readme

@marcdumais-work
Copy link
Contributor

noticed however that there's a mismatch between the plugins of the electron and browser app. In particular the 'vscode.git' extension stood out to me, as I recall it might be broken in Theia (at least for the electron version).

That's correct. I can confirm that the browser version is affected by the same problem:
eclipse-theia/theia#12173

@jfaltermeier
Create Browser Application and Dockerfile #75
1181a8b 
* Update repository structure and adapt headings in readme
* Align used plugins
* Add browser app start instructions
* add rate limits for plugin downloads
@jfaltermeier jfaltermeier marked this pull request as ready for review March 7, 2023 08:42
@jfaltermeier
Create Browser Application and Dockerfile #75
ef68982 
* Remove updater from browser app
@AlexandraBuzila
Copy link
Contributor

Thank you for the update! The changes look good and work well for me.

@jfaltermeier
Create Browser Application and Dockerfile #75
c8b5e3b 
* Try to fix javascript out of heap errors in windows workflow
@jfaltermeier
Copy link
Contributor Author

Regarding the windows verification Job. This was failing with "out of heap" errors on the github verification Jobs. Increasing the heap using the node options did not help. It was working locally in my Windows VM however.
My solution is to stop running the prepare-step in parallel, so the browser and electron app are built after each other now.

@jfaltermeier jfaltermeier merged commit a121e4f into master Mar 8, 2023
@jfaltermeier jfaltermeier deleted the jf/docker-application branch March 8, 2023 10:22
@marcdumais-work
Copy link
Contributor

marcdumais-work commented Mar 17, 2023 •
edited
Loading

FYI, the issue with vscode built-in git is supposed to be fixed, so we could switch to using it, if we want: eclipse-theia/theia#12295

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcdumais-work marcdumais-work marcdumais-work left review comments

@AlexandraBuzila AlexandraBuzila AlexandraBuzila approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jfaltermeier @marcdumais-work @AlexandraBuzila