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

Regression: docker and docker-compose versions empty in ddev version #1332

Merged
merged 7 commits into from
Dec 15, 2018

Conversation

rfay
Copy link
Member

@rfay rfay commented Dec 14, 2018

The Problem/Issue/Bug:

I noted that ddev version had empty lines for docker and docker-compose versions (recently added).

How this PR Solves The Problem:

Use the getter for them instead of using the variable directly.

Sad dance to resolve import cycles. Our packages are kind of out of control.

Manual Testing Instructions:

Use ddev version

Automated Testing Overview:

Added trivial stuff in the related test functions.

Related Issue Link(s):

Release/Deployment notes:

versionInfo["docker"] = DockerVersion
versionInfo["docker-compose"] = DockerComposeVersion
if versionInfo["docker"], err = GetDockerVersion(); err != nil {
_ = fmt.Errorf("failed to GetDockerVersion(): %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

These error values are formatted, but never returned.

@rfay rfay force-pushed the 20181214_fix_version_dockerinfo branch from 78d5b28 to a2ee615 Compare December 14, 2018 21:49
Copy link
Contributor

@andrewfrench andrewfrench left a comment

Choose a reason for hiding this comment

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

Improved error management and package updates look good, 👍 from me

@rfay rfay force-pushed the 20181214_fix_version_dockerinfo branch from a2ee615 to 0efcdee Compare December 15, 2018 05:51
@rfay rfay merged commit 21c7d6d into ddev:master Dec 15, 2018
@rfay rfay deleted the 20181214_fix_version_dockerinfo branch December 15, 2018 13:51
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