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

Added Docker Setup Option #1121

Merged
merged 24 commits into from Feb 2, 2019
Merged

Conversation

kristinyim
Copy link
Collaborator

Instructions for setup is at docker-dev.txt
TODO: Add these instructions to readthedocs

@kristinyim kristinyim closed this Jan 18, 2019
@kristinyim kristinyim deleted the docker branch January 18, 2019 22:48
Copy link
Collaborator

@felixzhuologist felixzhuologist left a comment

Choose a reason for hiding this comment

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

this looks great!

docker-dev.txt Outdated Show resolved Hide resolved
build/semesterly-base/Dockerfile Outdated Show resolved Hide resolved
@kristinyim kristinyim restored the docker branch January 28, 2019 14:21
@kristinyim kristinyim reopened this Jan 28, 2019
@kristinyim
Copy link
Collaborator Author

kristinyim commented Jan 29, 2019

@felixzhuologist could you take another look? I added this setup option to the Readthedocs as well


ADD ./requirements_base.txt /tmp

RUN apt-get install -y \
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this work? it looks like install and update are mixed up and i would expect to need to use && to run multiple commands. what would make the most sense to me would be to have a different run command/layer for each separate type of thing you're installing (debian package, node, python requirements):

RUN apt-get update -y && apt-get install ...
RUN curl .. && apt-get install -y nodejs
RUN pip install -r ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sorry, i didn't push the right version - its up now with &&

Copy link
Collaborator

Choose a reason for hiding this comment

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

last nit: can you separate the node and python commands into separate RUN instructions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I did what you wanted - you're talking about the last 3 lines right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah lgtm, thanks!

@kristinyim kristinyim merged commit 128296b into noahpresler:master Feb 2, 2019
@kristinyim kristinyim deleted the docker branch March 10, 2019 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants