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

Add Dockerfile for WebIDE #123

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ninoseki
Copy link

This PR adds Dockerfile for WebIDE.
Using Docker is one of the most convenient ways to run an app. I believe providing the official Dockerfile helps a lot.

@KOLANICH
Copy link

KOLANICH commented Dec 30, 2020

Usage of Docker for everything is just a cargo cult. If Google uses it for its highload services it doesn't mean that you should use it. If it should be used or not depends on a lot of factors. It is more like a prescribtion-use medicine rather than a silver bullet.

Comment on lines +6 to +7
&& apk add git \
&& git clone --recursive https://github.com/kaitai-io/kaitai_struct_webide /app \
Copy link
Member

Choose a reason for hiding this comment

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

  1. I don't understand why are you installing git into the container and then cloning the repo from inside, when this Dockerfile is part of the cloned repository (meaning that the kaitai_struct_webide repo has already been cloned)?

  2. Another problem is that you'll always get the main branch (master) by doing this, which will make it impossible to use this Dockerfile for Web IDE development (like building the image from locally changed files).

    If fetching the remote master branch was your intent, may I ask what's the point of running the Web IDE server with Docker instead of just going to https://ide.kaitai.io/devel/ (or alternatively fetching git clone https://github.com/kaitai-io/ide-kaitai-io.github.io and running a local server in the /devel/ folder).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants