Refactored build to separate browser and electron and added docker build for main #74
Conversation
harmen-xb
commented
Nov 25, 2024
•
harmen-xb
commented
- Refactored main build script to have browser and electron build (incl. symlink) to have minimize build for build and docker.
- Updated all versions to 0.0.0.
- Created Dockerfiles for ubuntu and alpine and added/updated build scripts
- Setup docker build in main pipeline, using the alpine Dockerfile.
…or CrossModel. Updated package configurations and webpack configuration.
Added separate protocol build, since this needs to happend first. Updated docker build to publish to GitHub container registry.
…nspec where used. Updated rimraf in all package.json files to also cleanup local node_modules.
Created alpine Dockerfile.
Added execution of package:extensions and copy of vsix to browser plugins folder.
Moved docker build to main.
Added build:dev and symlink:browser to main package json for browser development and symlink of ext. Remove prepare script everywhere. Updated build-and-test action to use build:dev.
…art. Updated README.
|
@martin-fleck-at hereby the PR for Docker container. I removed the rimraf on node_modules as discussed. I also made two build options for browser and electron to only build what is necessary. This seems to speed up the build quite a bit, since we only use browser anyway in build I think this make sense. |
There was a problem hiding this comment.
The change looks good to me overall and I like the unification of the version numbers as well as the splitting between browser and electron app. I built and ran the Docker image locally and it seemed to work as expected. I just have a few minor comments which maybe could be addressed.
Added Docker packaging in README.
|
@martin-fleck-at I processed your comments. Let me know if you approve :). |
There was a problem hiding this comment.
All my concerns have been addressed, thank you for the quick turnaround, Harmen! I'll ask Johannes tomorrow to just have a very quick look at it in case he spots something odd but from my side we can already merge this and fix any unexpected issues in a follow-up.
| @@ -0,0 +1,91 @@ | |||
| # Stage 1: Builder stage | |||
| FROM node:20-bookworm AS build-stage | |||
There was a problem hiding this comment.
Just realized that this is not actually Ubuntu but Debian, at least according to https://hub.docker.com/_/node:
Some of these tags may have names like bookworm or bullseye in them. These are the suite code names for releases of Debian and indicate which release the image is based on.
This is not a problem, just makes the file naming a bit confusing. Sorry, I missed that yesterday!
There was a problem hiding this comment.
Ah yes, good point. I also realized that a while ago, but forgot:). I will update to the file name and readme.
