Skip to content

Conversation

@moysesb
Copy link

@moysesb moysesb commented Mar 29, 2015

This fixes issue #1194 and is a prerequisite for implementing #1209

Copy link

Choose a reason for hiding this comment

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

Could we instead call resolve_build_path from process_container_options? I think that'd be conceptually cleaner.

Copy link
Author

Choose a reason for hiding this comment

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

Will do. It sure looks better in process_container_options.

@aanand
Copy link

aanand commented Mar 30, 2015

Thanks so much! Looks really good. Only some small changes to make - see commit comments.

Copy link

Choose a reason for hiding this comment

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

You can use the expand_path helper here, like get_env_files and resolve_host_path do.

@aanand
Copy link

aanand commented Mar 30, 2015

Almost there. Once you've done that last one, could you rebase and squash? Thanks.

@moysesb
Copy link
Author

moysesb commented Mar 31, 2015

All tests passed locally, the failure seems to be unrelated to my changes. Is there something I can do?

@aanand
Copy link

aanand commented Mar 31, 2015

Nope, was a Jenkins hiccup. Passing now.

LGTM

@aanand
Copy link

aanand commented Mar 31, 2015

Ah, needs a rebase thanks to #1226 - sorry!

@moysesb
Copy link
Author

moysesb commented Mar 31, 2015

Shifted BuildPathTest down to be the last class in the file, as it should be ;)

@aanand
Copy link

aanand commented Mar 31, 2015

Thanks. Could you rebase to the current master? I'm seeing confusing extra stuff in the tests diff.

* This fix introduces one side-effect: the build parameter is now
validated early, when the service dicionary is first constructed.
That leads to less scary stack traces when the path is not valid.

* The tests for the changes introduced here alter the fixtures
of those (otherwise unrelated) tests that make use of the 'build:'
parameter)

Signed-off-by: Moysés Borges Furtado <moyses.furtado@wplex.com.br>
@moysesb
Copy link
Author

moysesb commented Mar 31, 2015

My bad, I should have run the diff myself before pushing, should be good now.

@aanand
Copy link

aanand commented Mar 31, 2015

⭐ LGTM ⭐

@aanand
Copy link

aanand commented Mar 31, 2015

ping @dnephin - last bugfix for 1.2, so I'll roll another RC after this is merged!

Copy link

Choose a reason for hiding this comment

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

if working_dir is required it really shouldn't be a kwarg, it should be just a regular positional argument

@dnephin
Copy link

dnephin commented Apr 1, 2015

Just a minor style nit, nothing that would block a merge.

LGTM

dnephin added a commit that referenced this pull request Apr 1, 2015
Make value of 'build:' relative to the yml file.
@dnephin dnephin merged commit 0f70b86 into docker:master Apr 1, 2015
aanand pushed a commit to aanand/fig that referenced this pull request Apr 1, 2015
Make value of 'build:' relative to the yml file.
(cherry picked from commit 0f70b86)
aanand pushed a commit to aanand/fig that referenced this pull request Apr 1, 2015
Make value of 'build:' relative to the yml file.
(cherry picked from commit 0f70b86)

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
@moysesb moysesb deleted the relative_build branch April 3, 2015 12:57
yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015
Make value of 'build:' relative to the yml file.
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015
Make value of 'build:' relative to the yml file.
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
StefanScherer added a commit to hypriot/compose that referenced this pull request Apr 19, 2015
* tag '1.2.0': (46 commits)
  Bump 1.2.0
  Merge pull request docker#1278 from albers/completion-run-user
  Merge pull request docker#1261 from aanand/fix-vars-in-volume-paths
  Merge pull request docker#1202 from aanand/jenkins-script
  Merge pull request docker#1213 from moysesb/relative_build
  Merge pull request docker#1226 from aanand/merge-multi-value-options
  Merge pull request docker#1225 from aanand/fix-1222
  Fix regression in Dns and DnsSearch settings
  Fix volume merging when there's no explicit host path
  Fix service dict merging when only one dict has a volumes key
  Update docker-py and requests version ranges
  Revert "Remove restriction for requests version, update docker-py requirement"
  Revert "Use dev version of Docker"
  Revert "Add 'labels:' config option"
  Add 'labels:' config option
  Use dev version of Docker
  Make volume host paths relative to file, merge volumes when extending
  Implement `extends`
  Specify all HostConfig at create time
  Add build status \o/
  ...

Conflicts:
	Dockerfile
	README.md
	script/build-linux
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants