close
Skip to content

Fix loader error with different build syntax - #544

Merged
dnephin merged 1 commit into
docker:masterfrom
vdemeester:fix-build-loading
Sep 20, 2017
Merged

Fix loader error with different build syntax#544
dnephin merged 1 commit into
docker:masterfrom
vdemeester:fix-build-loading

Conversation

@vdemeester

Copy link
Copy Markdown
Collaborator

build: . was not working anymore. Fixing this by adding a new
tranform function for BuildConfig.

/cc @cdrage

Signed-off-by: Vincent Demeester vincent@sbr.pm

`build: .` was not working anymore. Fixing this by adding a new
tranform function for BuildConfig.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@codecov-io

codecov-io commented Sep 20, 2017

Copy link
Copy Markdown

Codecov Report

Merging #544 into master will increase coverage by <.01%.
The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master     #544      +/-   ##
==========================================
+ Coverage   49.06%   49.07%   +<.01%     
==========================================
  Files         200      200              
  Lines       16438    16447       +9     
==========================================
+ Hits         8066     8072       +6     
- Misses       7952     7955       +3     
  Partials      420      420

@thaJeztah

Copy link
Copy Markdown
Member

Do we know where it broke, and if this needs to be back ported?

@vdemeester

vdemeester commented Sep 20, 2017

Copy link
Copy Markdown
Collaborator Author

@thaJeztah from #481. As we don't load it anyway in docker/cli we don't fails there. But using it as a lib, it fails 😓

@cdrage

cdrage commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

@vdemeester kinda odd since build is yet to be supported in docker/cli compose, right? or did I miss a recent commit?

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vdemeester

Copy link
Copy Markdown
Collaborator Author

@cdrage it's just that your PR didn't take into account Load and thus didn't take into account that both things below works 👼. I missed that when reviewing the first one 👼

build: .
build:
  context: .
  dockerfile: foo.dockerfile

@cdrage

cdrage commented Sep 20, 2017

Copy link
Copy Markdown
Contributor

@vdemeester Doh. I forgot about that too.

Thanks for the fix 👍

@dnephin dnephin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dnephin
dnephin merged commit 09c8f47 into docker:master Sep 20, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.10.0 milestone Sep 20, 2017
@vdemeester
vdemeester deleted the fix-build-loading branch September 21, 2017 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants