Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that make and docker share the same image #1266

Merged
merged 1 commit into from May 5, 2018

Conversation

c-w
Copy link
Contributor

@c-w c-w commented May 5, 2018

Currently the make docker-build command creates a custom tagged image called cs61a/ok-server. The make docker-test command, on the other hand, delegates to docker-compose which will assign its own image name based on the service name declared in the compose file (web).

This means that in some situations docker-compose will unnecessarily rebuild the image. Specifying an explicit image name in the compose file eliminates this inconsistency and ensures that both make recipes (make docker-build and make docker-test) will always use the same image.

Currently the `make docker-build` command creates a custom tagged image
called `cs61a/ok-server`. The `make docker-test` command, on the other
hand, delegates to docker-compose which will assign its own image name
based on the service name declared in the compose file (`web`).  This
means that in some situations docker-compose will unnecessarily rebuild
the image. Specifying an explicit image name in the compose file
eliminates this inconsistency and ensures that both make recipes will
always use the same image.
Copy link
Member

@colinschoen colinschoen left a comment

Choose a reason for hiding this comment

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

LGTM

@colinschoen colinschoen merged commit 90c5a99 into okpy:master May 5, 2018
@c-w c-w deleted the enhancement/c-w/share-docker-image branch May 7, 2018 01:22
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.

None yet

2 participants