-
Notifications
You must be signed in to change notification settings - Fork 5.9k
make docker build working with after a fresh checkout #72
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
Conversation
@@ -224,7 +224,7 @@ const ensureCloned = register("vscode:clone", async (runner) => { | |||
} | |||
|
|||
runner.cwd = vscodePath; | |||
const checkout = await runner.execute("git", ["checkout", "tags/1.31.1"]); | |||
const checkout = await runner.execute("git", ["checkout", "tags/1.31.1"]); //TODO: this tag should come from a parameter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by a parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess he means it shouldn't be hard coded.
@@ -1,4 +1,4 @@ | |||
FROM node:8.15.0 | |||
FROM node:8.9.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change?
@@ -18,7 +18,7 @@ RUN yarn && yarn task build:server:binary | |||
# We deploy with ubuntu so that devs have a familiar environemnt. | |||
FROM ubuntu:18.10 | |||
WORKDIR /root/project | |||
COPY --from=0 /src/packages/server/cli-linux /usr/local/bin/code-server | |||
COPY --from=0 /src/packages/server/cli-linux-x64 /usr/local/bin/code-server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik, there shouldn't be a -x64
suffix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be - that is actually the only output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. This was recently changed without me knowing. It's fixed in #109.
I don't understand what this PR fixes. How does |
See #72 |
This PR makes the Dockerfile actually build without error :) |
No description provided.