-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update ARG, ENV and LABEL reference definitions #5434
Update ARG, ENV and LABEL reference definitions #5434
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.
LGTM but we should also update it for LABEL
… KV-pairs are required but additional ones are optional Signed-off-by: Shaun Thompson <[email protected]>
2f0a2ae
to
5379646
Compare
Added |
Ah, thanks! I had the tab still open as reminder, but didn't get round to opening a PR. Changes LGTM; one more thing (but doesn't have to be in this PR) I was considering, but I still had to read up on the current docs, is to make sure we properly outline the distinction between "value is set" and "no value" (
docker build --progress=plain -<<'EOF'
# syntax=docker/dockerfile:1
# nginx:alpine defines a NGINX_VERSION env-var
FROM nginx:alpine
ARG FOO_UNSET
ARG FOO_EMPTY=
ARG NGINX_VERSION
RUN printenv | grep -E '(FOO|NGINX)'
EOF #5 [2/2] RUN printenv | grep -E '(FOO|NGINX)'
#5 0.238 FOO_EMPTY=
#5 0.238 NGINX_VERSION=1.27.2 docker build --build-arg NGINX_VERSION=override --progress=plain -<<'EOF'
# syntax=docker/dockerfile:1
# nginx:alpine defines a NGINX_VERSION env-var
FROM nginx:alpine
ARG FOO_UNSET
ARG FOO_EMPTY=
ARG NGINX_VERSION
RUN printenv | grep -E '(FOO|NGINX)'
EOF #5 [2/2] RUN printenv | grep -E '(FOO|NGINX)'
#5 0.221 FOO_EMPTY=
#5 0.221 NGINX_VERSION=override |
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.
LGTM
Follow-up to suggestion from @thaJeztah in #5427
Update ARG and ENV syntax definitions to clarify that first KV-pair is required but additional ones are optional