-
Notifications
You must be signed in to change notification settings - Fork 345
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
docker: remove unused build tools #1060
docker: remove unused build tools #1060
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.
I don't agree with the user-facing change proposed here, but if those APK packages in the builder phase are unnecessary, happy to accept that change on its own.
Dockerfile
Outdated
@@ -12,7 +12,14 @@ LABEL maintainer=terraform-linters | |||
|
|||
RUN apk add --no-cache ca-certificates | |||
|
|||
COPY --from=builder /tflint/dist/tflint /usr/local/bin | |||
# copy tflint from builder |
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.
Remove these comments, they are unnecessarily re-stating things that can be readily inferred from the code. Everything happening here is basic and well-documented upstream. You can assume basic understanding of Dockerfiles here and not try to explain how they work.
Dockerfile
Outdated
WORKDIR /data | ||
|
||
# set the default entrypoint -- when this container is run, use this command | ||
ENTRYPOINT [ "tflint" ] |
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 the formatting consistent with Docker's docs, meaning no array padding:
Dockerfile
Outdated
ENTRYPOINT [ "tflint" ] | ||
|
||
# as we specified an entrypoint, this is appended as an argument (i.e., `tflint --help`) | ||
CMD [ "--help" ] |
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 this really the best default command, as opposed to invoking TFLint with no arguments, which is implicitly the same as tflint .
?
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.
reverting back to original behaviour
9632e82
to
df50044
Compare
Signed-off-by: Pujan Shah <pujan14@gmail.com>
df50044
to
7eac73c
Compare
Signed-off-by: Pujan Shah pujan14@gmail.com
Removed unused build tools from dockerfile and added default argument to image with CMD