-
Notifications
You must be signed in to change notification settings - Fork 617
docs: bake guides and refactor reference #1140
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
Conversation
a54ae12 to
3430de8
Compare
dockertopia
left a comment
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.
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
tonistiigi
left a comment
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.
If you want, you can merge as-is and follow up on the next PRs.
| ## Next steps | ||
|
|
||
| * [File definition](file-definition.md) | ||
| * [Configuring builds](configuring-build.md) |
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.
This doesn't go in the correct order atm. "Configuring builds" starts with an example using variables but variables have not been explained yet.
My suggestion is that "configuring builds" would start with examples about variables and also contains attrs.
Next topic is "User defined HCL functions"
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.
We define variables in "File definition" so I think it's fine? But the "HCL variables and functions" doesn't look quite right and should only be about HCL user-defined functions as you said. I made some changes about it.
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.
grouping variables with "configuring" and functions with the rest of the function section makes more sense to me. Yeah, the title should be just HCL functions as there are stdlib functions as well(should we list them?).
| title: "Configuring builds" | ||
| keywords: build, buildx, bake, buildkit, hcl, json | ||
| --- | ||
|
|
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.
This should start with an intro, e.g. "Bake supports loading build definition from files. But sometimes you need even more flexibility to configure this definition. For this use case, you can define variables inside the bake files that can be set by the user with environment variables or by attribute definitions in other bake files. If you wish to change a specific value for a single invocation you can use a --set flag."
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.
Added an intro and also moved some bits from HCL user-defined funcs specific to build configuration in this page.
docs/guides/bake/compose-xbake.md
Outdated
| @@ -0,0 +1,125 @@ | |||
| --- | |||
| title: "Extension field with Compose" | |||
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.
Let's just call it "Building Compose files" and add an extra intro block explaining that you can just build compose files directly and how each service definition inside the compose files maps to a target (could have a simple example for this as well).
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.
There is already a basic example in "File definition" for compose files. Also in "Target" specification we already have a note saying "In the case of compose files, each service corresponds to a target.".
This "Extension field with Compose" page is very specific to the case where some fields are not supported in the compose spec.
Do you think we should move the Compose file section to this page so we don't have many things around about compose but on the same page?
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 think this provides enough context for the new reader. They already need to know that they are looking for x-bake to find this useful. You can leave the example in the file definition but there is much more to explain in here.
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.
Added some context for building compose files. In a follow-up we could add new use cases to show how to use compose build fields from the specification and how they are translated to target with bake (secrets, cache).
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
tonistiigi
left a comment
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.
👍 Let's get this in and iterate from here.
follow-up #928 (review)
creates guides for bake as our reference docs for this command is quite huge and I think bake deserves its own section.
bake guides will be then added on docs web site, see docker/docs#14905. Preview: https://deploy-preview-14905--docsdocker.netlify.app/build/bake/file-definition/
I plan to add more concrete examples and use cases. let me now if the structure sounds good to you. cc @tonistiigi @dockertopia
Signed-off-by: CrazyMax crazy-max@users.noreply.github.com