Skip to content
This repository has been archived by the owner on Mar 7, 2018. It is now read-only.

Dockerize #107

Merged
merged 1 commit into from
Nov 28, 2017
Merged

Dockerize #107

merged 1 commit into from
Nov 28, 2017

Conversation

c-w
Copy link
Contributor

@c-w c-w commented Nov 28, 2017

@c-w c-w requested a review from jcjimenez November 28, 2017 20:58
Copy link

@jcjimenez jcjimenez left a comment

Choose a reason for hiding this comment

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

LGTM

FROM node:6.12.0

ADD . /app
WORKDIR /app

Choose a reason for hiding this comment

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

I'm wondering if instead of doing the ADD+WORKDIR thing, we do something with volumes: like this in the docker-compose.yml file instead:

    ports:
        - 8888:80
    volumes:
      - ./:/app

This way, any changes done from the container can be used or committed to git. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds great. I don't have any experience with using volumes, so I'd appreciate a PR from which I can learn :)

@c-w c-w merged commit 5eea24b into master Nov 28, 2017
@c-w c-w deleted the dockerize branch November 28, 2017 21:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants