Skip to content

Conversation

@dnephin
Copy link
Contributor

@dnephin dnephin commented Jul 13, 2017

@codecov-io
Copy link

codecov-io commented Jul 13, 2017

Codecov Report

Merging #333 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #333   +/-   ##
=======================================
  Coverage   48.85%   48.85%           
=======================================
  Files         186      186           
  Lines       12413    12413           
=======================================
  Hits         6064     6064           
  Misses       5975     5975           
  Partials      374      374


To use the default value of an `ARG` declared before the first `FROM` use an
`ARG` instruction without a value:
An `ARG` declared before a `FROM` is outside of a build stage, so it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe better to mention that every stage has individual args, then it is better to understand that "arg-in-from" is not an exception but just follows the rule.

How about this example:

ARG VERSION=latest
FROM busybox:$VERSION
ARG VERSION
RUN echo $VERSION > image_version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like "An ARG instructions is only relevant for the next FROM instruction that comes after it in the Dockerfile, and then using this example to illustrate that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like "An ARG instructions is only relevant for the next FROM instruction that comes after it in the Dockerfile,

That's not exactly correct. There are two cases:

  • any ARG before the first FROM can be used in any FROM line
  • any ARG within a build stage (after a FROM) can be used in that build stage

I can add more details to the ARG reference section as well (this stuff is under FROM) , but I think this line needs to be updated as well something along the lines of what I have.


To use the default value of an `ARG` declared before the first `FROM` use an
`ARG` instruction without a value:
An `ARG` declared before a `FROM` is outside of a build stage, so it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like "An ARG instructions is only relevant for the next FROM instruction that comes after it in the Dockerfile, and then using this example to illustrate that.

@dnephin
Copy link
Contributor Author

dnephin commented Jul 13, 2017

I've updated to include more details, and added some more structure to the ARG section, PTAL

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The text lgtm but the examples could be better. The image_version example was specifically for the case where it may make sense to use the same variable in FROM and inside the stage. I would switch the examples and make "arg-in-from" use the image_version and arguments in multiple stages use ARG SETTINGS.

dnephin added 2 commits July 14, 2017 16:55
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin dnephin force-pushed the docs-arg-in-from-take-2 branch from e162210 to ad5d035 Compare July 14, 2017 20:59
@dnephin
Copy link
Contributor Author

dnephin commented Jul 14, 2017

Makes sense, I've swapped the examples.

@tonistiigi
Copy link
Member

LGTM

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants