Build proto in Docker #173
Conversation
stephanos
force-pushed
the
proto-docker
branch
7 times, most recently
from
63be1fd to
8ddfeee
Compare
| cd /workspace/workers/typescript | ||
| npm install && npm run proto-gen |
There was a problem hiding this comment.
Easy to forget ...
| - `protoc` and `protoc-gen-go` must be installed | ||
| - tip: don't worry about the specific versions here; instead, the GitHub action will make a diff | ||
| available for download that you can use with `git apply` |
There was a problem hiding this comment.
Might be worth changing this to say you either need proto or docker to get kitchen sink going locally
| -t "${IMAGE_NAME}" "${PROJECT_ROOT}" | ||
|
|
||
| echo "Running kitchen-sink-gen build..." | ||
| docker run --rm -v "${PROJECT_ROOT}:/workspace" -w /workspace "${IMAGE_NAME}" bash -c " |
There was a problem hiding this comment.
Not a huge fan of mounting the whole project dir into the docker image. This can make for slow loads as it copies everything, and what it writes back out often has weird, incorrect permissions.
That said, this is still easier than before, so I wouldn't block on this, but, if possible might be nice to cleanly copy the result files out.
There was a problem hiding this comment.
Hm, I spent almost an hour on this and it's harder than I thought: I don't want to copy all files into the container (node_modules is large) which requires some filtering, I don't want to specify every single output because that's brittle, but copying all files out of the container also defeats the purpose, doing a diff is harder than it seems, ...
I'll merge for now and we can discuss improvements.
What was changed
(1) Provides script to build protos locally.
(2) Moves all version identifier to file.
Why?
(1) Allow contributors to build correct protos locally.
(2) Single source of truth.
Checklist
Closes
How was this tested: