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 and docker-compose #345

Closed
wants to merge 3 commits into from

Conversation

stefanocudini
Copy link
Contributor

@stefanocudini stefanocudini commented Aug 24, 2020

I'm created a new PR based to node docker image

many of the points from the previous discussion have been solved

#327

@stefanocudini
Copy link
Contributor Author

docker-compose.yml is optional, it's only an example for usage image under production environment..

@stefanocudini stefanocudini mentioned this pull request Sep 1, 2020
@nilsnolde
Copy link

I'm not with the project anymore actually. However, a quick glance at the dockerfile suggests that you actually only addressed the image thing. The other points are still a problem. Copied from the original discussion:

  • too many RUN statements (each RUN adds another layer to the final image, only useful if you're having multiple images share a certain set of base commands, not useful here..)
  • no LABEL like maintainer
    - Ubuntu base image is not ideal, e.g. node:12.13.1-alpine or similar is better
  • also there's no need to git clone the repository: you're already inside it with Docker (this is pretty important)

Also:

  • either remove the commented out statements or explain why they could/should be uncommented
  • let's not mix YAML and JSON notation, i.e. lists in docker-compose, rather like this:
ports: 
- 8080:8080

@TheGreatRefrigerator TheGreatRefrigerator changed the base branch from master to development December 19, 2020 23:06
@TheGreatRefrigerator
Copy link
Contributor

Hi @stefanocudini ,

sorry for the long wait.

I've finally taken some time to do this.

Please take a look at this branch

It would be great if you could test it out and tell me if it works with a local ors backend (i don't have one installed and it would take me quite some time to do that i suppose..)

The app deployment should now be pretty straight forward using docker-compose.

I think also developing should work due to the watches and volumes passed in docker-compose.

Hopefully also this will also help our backend guys to quickly pair a client with their development environment :)

Hope to hear from you

@TheGreatRefrigerator
Copy link
Contributor

TheGreatRefrigerator commented Dec 19, 2020

A little info to the change:

  • using other image with git + node preinstalled
  • avoid cloning the repo, instead copying needed files & paths to the docker container
  • using app folder as volume (not sure if the single file volume for Gruntfile.default works. Docker sometimes acts strange with single files)
  • Add changelog & readme
  • move command to docker-compose so building and running is separated
  • based on current development branch including ng-slider cleanup change which makes installing and running less "breaking"

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.

None yet

3 participants