Using ENTRYPOINT exec form in Dockerfile
#88
Conversation
Dockerfile's ENTRYPOINT exec form is the preferred form over shell form. This allows user of this docker image to specify flags as CMD. With this change, I can now add flags directly to the end like this: `docker run -u "$(id -u):$(id -g)" -w=/tmp -v "$PWD":/tmp nvuillam/npm-groovy-lint --verbose --noserver --no-insight --failon error`. Whereas before, I'd have to override your ENTRYPOINT with `--entrypoint=npm-groovy-lint` which is not as clean.
Codecov Report
@@ Coverage Diff @@
## master #88 +/- ##
=======================================
Coverage 93.71% 93.71%
=======================================
Files 72 72
Lines 1798 1798
=======================================
Hits 1685 1685
Misses 113 113 Continue to review full report at Codecov.
|
|
I agree that it would be nice to be able to send arguments directly in docker run :) Questions:
@davegallant , as you built the v1 of the dockerfile, maybe you have comments too ? |
nvuillam
added
enhancement
| @@ -10,4 +10,5 @@ RUN apk add --update \ | |||
|
|
|||
| RUN npm i -g | |||
|
|
|||
| ENTRYPOINT "npm-groovy-lint" | |||
| ENTRYPOINT ["npm-groovy-lint"] | |||
There was a problem hiding this comment.
Ooh, this was shell form. https://docs.docker.com/engine/reference/builder/#cmd
Dockerfile
Outdated
| @@ -10,4 +10,5 @@ RUN apk add --update \ | |||
|
|
|||
| RUN npm i -g | |||
|
|
|||
| ENTRYPOINT "npm-groovy-lint" | |||
| ENTRYPOINT ["npm-groovy-lint"] | |||
| CMD ["--help"] | |||
There was a problem hiding this comment.
This differs from running npm-groovy-lint.
Should the default behaviour be changed?
There was a problem hiding this comment.
You're right. In order to avoid breaking the default behavior and potentially angering hordes of users who don't specify their docker tags, we can revert this. In my testing we can still do docker run -u "$(id -u):$(id -g)" -w=/tmp -v "$PWD":/tmp nvuillam/npm-groovy-lint --help to achieve the same result as long as the exec form is used.
|
Updated per feedback and should be ready to merge? |
|
Sorry i'm in vacation, i'll validate & generate version asap :) |
Fixes
DockerfileProposed Changes
docker run -u "$(id -u):$(id -g)" -w=/tmp -v "$PWD":/tmp nvuillam/npm-groovy-lint --verbose --noserver --no-insight --failon error. Whereas before, I'd have to override your ENTRYPOINT with--entrypoint=npm-groovy-lintwhich is not as clean.--help, so that if the user doesn't specify the CMD the help menu will be displayed. For most intent and purposes we expect the user to customize runtime by passing in their desired parameter so I feel this is a more sane default.