Add Dockerfile #29
Add Dockerfile #29
Conversation
|
@JCBird1012 @nelsonw2014 Not sure which one of you would rather review this, so go ahead if you want. |
Codecov Report
@@ Coverage Diff @@
## master #29 +/- ##
=======================================
Coverage 54.61% 54.61%
=======================================
Files 4 4
Lines 390 390
=======================================
Hits 213 213
Misses 151 151
Partials 26 26Continue to review full report at Codecov.
|
|
I got it |
Dockerfile
Outdated
| COPY . /go/src/github.com/kochman/hotshots | ||
| RUN go install github.com/kochman/hotshots | ||
|
|
||
| ENV HOTSHOTS_DIR /var/hotshots |
There was a problem hiding this comment.
This is predefined in code, we don't need to set the env var
There was a problem hiding this comment.
Also you need to make the directory /var/hotshots and make sure permissions work for hotshots, otherwise it won't start.
There was a problem hiding this comment.
I was thinking it would be better to be explicit about it, but I'll remove it. Also, /var/hotshots should be mounted into the container when it is run (I'll document this), so it will have open enough permissions by default.
There was a problem hiding this comment.
Okay, remove that line and add some form of documentation and I'll merge
|
We should at some point test the dockerfile actually starts in our travis build. For another time |
|
Yeah, good idea |
|
On a separate note, is there a way to make it so travis build is only required if *.go files are changed? |
|
Yeah, we could definitely do something with having the build ask git if there were any changes in *.go files in the branch. |
|
Also we should definitely stop it from building twice lol |
|
What do you mean by twice? |
|
there's two checks, one for a pr, the other for anything that is pushed. We should probably kill the pr one. |
|
I think they're both important. PR tests the resulting merge commit into master; push just checks the branch. |
|
That's fair, but usually merging into branch should be required before push to master |
Just use the default Go base Docker image. We could be a lot smarter about a multi-step builder so we don't have all of the source files laying around in the resulting Docker image, but this is fine for now.
The
.dockerignoreignores everything and then includes only what we need to actually compile and run the binary.