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

Fix volume merging #1160

Merged
merged 2 commits into from Mar 24, 2015
Merged

Fix volume merging #1160

merged 2 commits into from Mar 24, 2015

Conversation

aanand
Copy link

@aanand aanand commented Mar 23, 2015

There were a couple of silly mistakes in the volume merging code. Fixed, with regression tests.

@aanand aanand force-pushed the fix-volume-merging branch 2 times, most recently from ea82082 to 23dc4e7 Compare March 23, 2015 18:50
{'volumes': ['/bar:/code', '/data']},
)
self.assertEqual(set(service_dict['volumes']), set(['/bar:/code', '/data']))

Copy link

Choose a reason for hiding this comment

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

Normally this would be 6 separate tests, so the test names can describe the case that is being tested. It's not completely clear to which the difference between each of these without inspecting each in detail.

Copy link
Author

Choose a reason for hiding this comment

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

Agreed - I've split them out.

Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
Signed-off-by: Aanand Prasad <aanand.prasad@gmail.com>
@dnephin
Copy link

dnephin commented Mar 24, 2015

LGTM

aanand added a commit that referenced this pull request Mar 24, 2015
@aanand aanand merged commit 1f06070 into docker:master Mar 24, 2015
@aanand aanand deleted the fix-volume-merging branch March 24, 2015 20:40
yuval-k pushed a commit to yuval-k/compose that referenced this pull request Apr 10, 2015
Fix volume merging
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
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