Use the official docker images while building hygdrop #11
Conversation
Changes Dockerfile to use the hylang image instead of building hy itself, also requirements file modified to remove earlier dependancies of hy & astor
|
|
||
| ADD . /opt/hygdrop | ||
| WORKDIR /opt/hygdrop | ||
| RUN pip3 install -r requirements-docker.txt |
There was a problem hiding this comment.
Why not do something like:
COPY requirements-docker.txt /opt/hygdrop/
RUN pip3 install -r requirements-docker.txt
COPY . /opt/hygdropThat way we get to utilize the Docker build cache a little better.
There was a problem hiding this comment.
Ah, Didn't think of that.thanks! changing in a bit
There was a problem hiding this comment.
Also, since the hylang image already has Hy installed, can't we use the same requirements.txt and let the already-installed Hy satisfy the Hy dep?
There was a problem hiding this comment.
Yep.. that is why created requirements-docker which doesn't include hy.. the plain requirements contains hy if you're doing it the traditional way. WDYT?
There was a problem hiding this comment.
No, I mean to say that you don't need to split them, do you? Isn't pip smart enough to notice that the hy>=0.10 dep is already satisfied and skip it appropriately?
There was a problem hiding this comment.
Just to make sure I verified that assumption:
root@5006d933f5b6:/usr/src/python# pip install hy
Requirement already satisfied (use --upgrade to upgrade): hy in /opt/hylang/hy
...😄
- made installing from requirements first to utilize build cache better - removed redundant requirements
|
@tianon added the steps you said (& removed unnessary requirements file), let me know if this is ok |
|
LGTM |
Changes Dockerfile to use the hylang image instead of building hy
itself, also requirements file modified to remove earlier dependancies
of hy & astor
Ping @tianon @paultag @copyninja for review