Nim: Add info about compiling to JS and running. - #2682
Conversation
tianon
left a comment
There was a problem hiding this comment.
Looks fine to me -- do you want to wait for your other PR to merge this, or merge this as-is since it's correct even without image changes?
(also one non-blocking note/question below)
| RUN nim js -o:app.js src/app.nim | ||
|
|
||
| FROM node:latest | ||
| COPY --from=builder /usr/src/app/app.js . |
There was a problem hiding this comment.
FYI, this is encoding the implicit assumption of WORKDIR /usr/src/app in the nim image (which is fine, but something you should be aware of / be doing intentionally if you're going to do it, since it makes that value slightly "stickier" to document it this way)
There was a problem hiding this comment.
True, and I was somewhat conflicted with this line... Initially, there was WORKDIR /app in both the builder and the runner images, which does make things more explicit but also noisier.
I think that as an example that is intended to be modified for your needs (I mean, you probably don't have an app called app anyway, duh) this is fine.
There was a problem hiding this comment.
Looks fine to me -- do you want to wait for your other PR to merge this, or merge this as-is since it's correct even without image changes?
I'd rather have that merged now as is.
No description provided.