Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Fix the docker ps command #146

Merged
merged 1 commit into from
Jun 6, 2018
Merged

Fix the docker ps command #146

merged 1 commit into from
Jun 6, 2018

Conversation

parlt91
Copy link
Contributor

@parlt91 parlt91 commented Jun 5, 2018

Previously the docker psCmd function made use of getImageID
which expects a repo:tag combination, but was called with the
ID which the function was expected to return. Thus the call
of getImageID was redundant in this place.
The tests were updated to reflect the new behaviour.

Another problem that influenced zypper-docker ps was that
an image was not added to the outdated string in the cache
file after a zypper-docker up or patch if the image to be
updated was parsed with it's image ID.

Fixes: #144
Fixes: #145
Signed-off-by: Pascal Arlt parlt@suse.com

Copy link
Contributor

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

Only one minor nitpick. Thanks for the nice fixes!

helpers.go Outdated

return "", fmt.Errorf("Cannot find image %s", name)
return imageID, err
Copy link
Contributor

Choose a reason for hiding this comment

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

Although err == nil, please use nil in the return statement instead. It's easier to understand and a common pattern.

Previously the docker psCmd function made use of getImageID
which expects a repo:tag combination, but was called with the
ID which the function was expected to return. Thus the call
of getImageID was redundant in this place.
The tests were updated to reflect the new behaviour.

Another problem that influenced zypper-docker ps was that
an image was not added to the outdated string in the cache
file after a zypper-docker up or patch if the image to be
updated was parsed with it's image ID.

Fixes: SUSE#144
Fixes: SUSE#145
Signed-off-by: Pascal Arlt <parlt@suse.com>
@vrothberg vrothberg merged commit bc8e75c into SUSE:master Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants