-
Notifications
You must be signed in to change notification settings - Fork 476
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
CON-2774 Update DOM Dockerfile #2641
Conversation
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.
@lotoussa I left just some general comments. Could you please elaborate a bit on why you want to make these changes?
Besides that, this image is also used for part of our content for the JS part of the curriculum besides the Discovery Piscine (just for your information) :)
@@ -1,6 +1,24 @@ | |||
FROM buildkite/puppeteer:7.1.0 | |||
FROM alpine:latest |
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.
better to specify the version to avoid unexpected version conflicts in the future
RUN apk add --no-cache \ | ||
chromium \ | ||
nss \ | ||
freetype \ | ||
harfbuzz \ | ||
ca-certificates \ | ||
ttf-freefont \ | ||
nodejs \ | ||
yarn |
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.
Is there any specific reason for installing all these individually instead of using, for example, the docker image suggested in the official documentation?
|
||
WORKDIR /app | ||
COPY dom . |
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.
Keep in mind that his Dockerfile
needs to work for our GitHub Actions :)
The command used to build the image is here
|
@lotoussa should we close this PR? :) |
@nprimo hello yes i believe it's not relevant |
CON-2774
Why?
Update the DOM Dockerfile. This version was used in previous Discovery piscine in order to properly work, and is now push to the public official repository as it will be used for the IA Discovery Piscine.
Solution Overview
The Dockerfile content come from a copy paste of another docker file of the public repository, it has been tested already on previous Discovery Piscine and seem to work but still need a double check.