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

Dockerfile: include git #231

Merged
merged 3 commits into from Mar 13, 2019

Conversation

Projects
None yet
4 participants
@nhooyr
Copy link
Collaborator

nhooyr commented Mar 13, 2019

Also improves installation of runtime deps.

Closes #221
Closes #230
Closes #203

@nhooyr nhooyr requested review from ammario and kylecarbs Mar 13, 2019

@ammario

This comment has been minimized.

Copy link
Contributor

ammario commented Mar 13, 2019

I think we should merge in everyone else's PRs instead so they get attribution.

@pokemonlover1234

This comment has been minimized.

Copy link
Contributor

pokemonlover1234 commented Mar 13, 2019

Mine (#230) does the same thing, except I put the git install in the wrong spot.

Add git to docker container
Install git in the docker container, proposed by #203. While it is possible that issue requires further discussion, this is just a proposal pull request. If that proposal is accepted, here's the pull request.
@nhooyr

This comment has been minimized.

Copy link
Collaborator Author

nhooyr commented Mar 13, 2019

@ammario the changes in this PR are different from the other PRs. Regardless, I've included all their commits here so they still get attribution and then included my own commit at the end to fix things up.

@nhooyr nhooyr force-pushed the nhooyr:docker branch from 534730b to c8307a7 Mar 13, 2019

Dockerfile Outdated
@@ -1,6 +1,6 @@
FROM node:8.15.0

# Install VS Code's deps. These are the only two it seems we need.
# Install VS Code's deps. These are the only two it seems we need. Install git for source control.

This comment has been minimized.

@ammario

ammario Mar 13, 2019

Contributor

Second sentence is in the wrong place (and extraneous imo)

This comment has been minimized.

@nhooyr

nhooyr Mar 13, 2019

Author Collaborator

I agree, from old a commit I just added. Will remove.

@nhooyr nhooyr force-pushed the nhooyr:docker branch from c8307a7 to 87995ad Mar 13, 2019

@nhooyr nhooyr requested a review from ammario Mar 13, 2019

@nhooyr nhooyr force-pushed the nhooyr:docker branch from 87995ad to 8f139db Mar 13, 2019

Dockerfile: include git
Closes #221
Closes #230
Closes #203

@nhooyr nhooyr force-pushed the nhooyr:docker branch from 8f139db to 862c944 Mar 13, 2019

@nhooyr nhooyr merged commit e3d9716 into codercom:master Mar 13, 2019

@nhooyr nhooyr deleted the nhooyr:docker branch Mar 13, 2019

net-tools \
git \
locales
RUN locale-gen en_US.UTF-8

This comment has been minimized.

@SuperSandro2000

SuperSandro2000 Mar 13, 2019

Contributor

We could squash that into the other run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.