Add variable typing to reference docs #3189
Conversation
docs/bake-reference.md
Outdated
| } | ||
| ``` | ||
| ```console | ||
| $ VALS_JSON=$(jo -a hello with,comma 'with"quote') docker buildx bake |
There was a problem hiding this comment.
can we use the canonical form to make it more explicit?
| $ VALS_JSON=$(jo -a hello with,comma 'with"quote') docker buildx bake | |
| $ VALS_JSON='["hello","with,comma","withquote"]' docker buildx bake |
There was a problem hiding this comment.
How about if I did something like
$ cat data.json
["hello","with,comma","with\"quote"]
$ VALS_JSON=$(< data.json) docker buildx bakeIn the previous section for CSV I had suggested that CSV was (generally) better for interactive use, while JSON better for machine use. And also, that JSON was the answer for unusual data. I know jo may not be as familiar to readers as jq, so not immediately clear what I was trying to convey.
If I have that data coming from a file, that's a little more in the spirit of why this processing mode exists. Whether this file came from an API, generated from another tool, etc. doesn't really matter.
If you still prefer it the way you suggested, that's fine. But usually one wouldn't handcraft JSON like that, which is why I avoided it as well.
There was a problem hiding this comment.
while JSON better for machine use
Oh ok makes sense then we can keep it
There was a problem hiding this comment.
I still moved to the file approach. It slipped past me when I originally wrote it and when I was sanity checking yesterday, but I think jo can only output objects and arrays; anyone running the other example that you called out would find that it fails...
docs/bake-reference.md
Outdated
|
|
||
| # These are all equivalent | ||
| $ FOO_JSON_JSON='"bar"' docker buildx bake <...> | ||
| $ FOO_JSON_JSON=$(jo -a bar) docker buildx bake <...> |
There was a problem hiding this comment.
Same here
| $ FOO_JSON_JSON=$(jo -a bar) docker buildx bake <...> | |
| $ FOO_JSON_JSON='["bar"]' docker buildx bake <...> |
|
Even though this is a reference doc, it seemed like the amount of documentation devoted to it should be proportional to the anticipated frequency of use and its importance to building stuff. That resulted in total omission of some things (like the Questions/issues that have come up between the initial implementation and these docs:
|
I think current changes you made in reference docs are good. If we want more specific use cases that are out of scope for reference docs, we can define them in https://github.com/docker/docs/blob/main/content/manuals/build/bake/variables.md
As
Yes agree this feels like a bug. As discussed offline with @tonistiigi, we can have a either a quick patch if possible or just document that default value should be always set when a type is defined if you feel it needs more thoughts. |
Here's my proposal:
The current exception for (1) is the // before
typeSpecified = !varType.Equals(cty.DynamicPseudoType)
// after
typeSpecified = !varType.Equals(cty.DynamicPseudoType) || hcl.ExprAsKeyword(vr.Type) == "any"That change broke existing tests asserting the 'missing'/'any' equivalence. That's why it might make sense to simply ignore Based on previous comments, I'm guessing you'll be good with (1) and (3), and at least not disagree with (2), so I'll proactively proceed to create a separate PR (but obviously let me know if you disagree or have other proposals). I'll sidestep the "null as an override" question for the time being; if it even makes sense, and if it doesn't behave as expected, I'd think that would be a simple bug/request; the issue around (2) is much bigger than that. I created #3198 for this. |
This documents the variable typing introduced in docker#3167. Signed-off-by: Roberto Villarreal <rrjjvv@yahoo.com>
This documents the variable typing introduced in #3167.