model: refactor static build properies onto m.buildInfo.buildDetails [ch1136]#879
Conversation
|
|
||
| type DockerBuildArgs map[string]string | ||
| func (m Manifest) DockerInfo() DockerInfo { | ||
| switch info := m.buildInfo.(type) { |
There was a problem hiding this comment.
would this be simpler as a plain type cast? not sure what the extra machinery of a switch is getting you
| f.TouchFiles(paths) | ||
|
|
||
| f.assertChild("parent", "sibling/fileD", "") | ||
| fmt.Println("---") |
There was a problem hiding this comment.
oh lol this wasn't supposed to be in here
| "github.com/windmilleng/tilt/internal/testutils/tempdir" | ||
| ) | ||
|
|
||
| func TestBullshit(t *testing.T) { |
There was a problem hiding this comment.
this either -- was testing weird ospath.Child hanging
| current := file | ||
| child := "." | ||
| for true { | ||
| fmt.Printf("checking dir '%s' for current '%s' (child = %s)\n", dir, current, child) |
|
|
||
| // TODO(maia): buildInfo | ||
| // Info needed to build an image. Can be StaticBuild, FastBuild... etc. | ||
| buildInfo buildInfo |
There was a problem hiding this comment.
is there some reason why this isn't a DockerInfo? afaik, there are no other types of buildInfos. it seems like a meaningless interface
There was a problem hiding this comment.
Fair. Theoretically in future we might want to use different image builders, but i agree that we don't need that flexibility built in right now.
| name := manifest.DockerRef() | ||
| if manifest.IsStaticBuild() { | ||
| dInfo := manifest.DockerInfo() | ||
| if dInfo.Empty() { |
There was a problem hiding this comment.
i wonder if ValidateDockerK8sManifest and this validation should be merged, so that ValidateDockerK8sManifest returns a guaranteed non-empty DockerInfo
There was a problem hiding this comment.
well this is no longer in there so mreh.
2 participants
(in the interest of not having this be a single giant code review --
will do FastBuild properties in a different PR.)
Hello @jazzdan, @nicks,
Please review the following commits I made in branch maiamcc/build-info:
3c8c237 (2018-12-20 19:33:07 -0500)
static build properties
9be6b3c (2018-12-20 18:27:29 -0500)
staticDf
c2b01fa (2018-12-20 18:12:26 -0500)
move docker stuff into nested field on manifest
Code review reminders, by giving a LGTM you attest that: