The Wayback Machine - https://web.archive.org/web/20260226204800/https://github.com/docker/cli/pull/544
Skip to content

Fix loader error with different build syntax#544

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

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

Conversation

@vdemeester
Copy link
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
Copy link

codecov-io commented Sep 20, 2017

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
Member

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

@vdemeester
Copy link
Collaborator Author

vdemeester commented Sep 20, 2017

@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
Copy link
Contributor

cdrage commented Sep 20, 2017

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

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@vdemeester
Copy link
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
Copy link
Contributor

cdrage commented Sep 20, 2017

@vdemeester Doh. I forgot about that too.

Thanks for the fix 👍

Copy link
Contributor

@dnephin dnephin left a comment

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