-
Notifications
You must be signed in to change notification settings - Fork 203
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
Docs: Semiautomatic platform ARGs #2084
Docs: Semiautomatic platform ARGs #2084
Conversation
The `bud` sub-command has been a hidden alias to `build` for quite a while. Fix the documentation accordingly. Signed-off-by: Chris Evich <[email protected]>
22e4582
to
69922d8
Compare
Note: I'm not entirely happy with the way the new header renders in markdown or as a man page. However, it's consistent with the rest of the document (which also renders incomplete/badly). For example anything surrounded by GT/LT characters (like |
@edsantiago and @rhatdan I'd love your feedback when you get a chance. This isn't critical, but would be nice to have 😄 |
Ephemeral COPR build failed. @containers/packit-build please check. |
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.
As someone unfamiliar with these variables, it took me a really long time to understand this wording. Would you mind retrying, trying to target a reader familiar with the intended use of these variables but not the magic "ARG xxx" syntax?
docs/Containerfile.5.md
Outdated
about the host performing the build, and the "TARGET" prefix represents | ||
the intended platform for the content. | ||
|
||
Each of the following ARG variables must be defined empty before use, |
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'm still unable to understand what "defined empty" means, even after many rereadings over several hours
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.
It's a really dumb (Docker) design. You literally put (for example) ARG TARGETARCH
in your Containerfile
. Forever after that, you can reference ${TARGETARCH}
and it will hold the value (for example) amd64
or ppc64le
depending on what platform the build is happening for.
I think this is a good suggestion, but I'm slightly confused with the wording. Do you mean, target a reader familiar with the use of |
All/most markdown renderers will mask any naked `<` and `>` characters that appear in the plain text. This was making the documented syntax of the `ARG` command unintelligible. Fix this. Signed-off-by: Chris Evich <[email protected]>
Available for quite some time, but often useful and undocumented. Fix that and a minor whitespace problem. Signed-off-by: Chris Evich <[email protected]>
69922d8
to
c314c91
Compare
Force-push: Hopefully this addresses Ed's concerns. |
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, thank you!
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cevich, edsantiago, flouthoc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/lgtm |
Available for quite some time, but often useful and undocumented.
Fix that and a minor whitespace problem.
Also (minor), rename "buildah bud" to "buildah build".
Fixes: #1935
Ref. Dockerfile docs