Fix issue in which extending could end up with both build and image #66
Conversation
ibuildthecloud
force-pushed
the
invalid-merge
branch
from
02e1a65 to
17c6ff7
Compare
According to Docker Compose docs > In the case of build and image, using one in the local service causes > Compose to discard the other, if it was defined in the original > service. The above was not being respected Signed-off-by: Darren Shepherd <darren@rancher.com>
ibuildthecloud
force-pushed
the
invalid-merge
branch
from
17c6ff7 to
26d8c30
Compare
|
ping @dnephin @vdemeester |
|
|
||
| if child.Image != "foo" { | ||
| t.Fatal("Invalid image", child.Image) | ||
| } |
There was a problem hiding this comment.
It looks like we already have testify.assert as a dependency.
What do you think about making all of these use assert.Equal() ?
There was a problem hiding this comment.
I'm not against testify if we already have it in our dependencies. @ibuildthecloud opinions ? 😺
There was a problem hiding this comment.
I'd really, really, really like to use go-check. I'm not a fan of testify and would like to move away from it. I did start with testify for this code. Are we cool with go-check? Engine uses go-check
There was a problem hiding this comment.
I believe engine uses go-check for integration tests, and has a mix of testify.assert and regular checks in unit tests.
Anything is better than the default t.Fatal() stuff. I like that testify.assert doesn't require you to change the test suite, it's just functions you can use with the default suite.
There was a problem hiding this comment.
Yes, go-check is more intrusive. But I can just use testify.assert. I'll change that now.
There was a problem hiding this comment.
Ok for go-check too 😸.
I can do a separate PR on migrating test to go-check then 😉.
|
LGTM, but I think it would be nice to make use of testify.assert |
|
@dnephin Going forward I'll use testify.assert. Rather not change something now and break it. |
Fix issue in which extending could end up with both build and image
|
cool, works for me |
According to Docker Compose docs
The above was not being respected