-
Notifications
You must be signed in to change notification settings - Fork 34
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
Better onboarding #161
base: main
Are you sure you want to change the base?
Better onboarding #161
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,15 +3,19 @@ | |
# https://hub.docker.com/_/golang | ||
FROM golang:1.13 as builder | ||
|
||
WORKDIR app/ | ||
WORKDIR /go/app/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is following best-practices of using an absolute path instead of relative. |
||
|
||
# Copy internal libraries. | ||
COPY . . | ||
# Copy and cache dependencies | ||
COPY ../go.mod . | ||
COPY ../go.sum . | ||
Comment on lines
-8
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Analyzing execution of Now with my proposed changes, the 2 first executions were very similar. But the 3rd execution, which has an updated file, took just 13s (vs 56s). Again, I'm not working often with the repo to say if that's important. But when I had to build the image several times, that saved me quite some time There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just for reference, running these before There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tyron yep I am on board with copying just the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW we can also mount the go build cache as well if you want to speed it up even more :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey, so I tested a bit more because I was curious on how my setup was working. I believe the notation in the Dockerfile is not based on your current pwd, yet relative to Dockerfile itself. From what I can test, even if your build context is the root folder and the go.mod is in the root folder, it should still be written as So since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, now I don't understand anything. Both |
||
|
||
# Retrieve application dependencies. | ||
# This allows the container build to reuse cached dependencies. | ||
# This allows the container build to reuse cached dependencies as long as go.mod and go.sum are unchanged. | ||
RUN go mod download | ||
|
||
# Copy internal libraries. | ||
COPY . . | ||
|
||
# Build the binary. | ||
RUN CGO_ENABLED=0 GOOS=linux go build -o doc -mod=readonly -v ./cmd/doc/main.go | ||
|
||
|
@@ -22,7 +26,7 @@ FROM alpine:3 | |
RUN apk add --no-cache ca-certificates | ||
|
||
# Copy the binary to the production image from the builder stage. | ||
COPY --from=builder go/app/doc ./ | ||
COPY --from=builder /go/app/doc ./ | ||
COPY ./template ./template | ||
COPY ./static ./static | ||
|
||
|
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.
Code updated below to use environment variables (so that this code can run in a docker aside the gitter runner), and these defaults were kept for backwards compatibility.