Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Fix remove image by Id #740

Merged
merged 2 commits into from
Nov 14, 2019
Merged

Conversation

jcsirot
Copy link
Contributor

@jcsirot jcsirot commented Nov 13, 2019

- What I did
Fix the docker app image rm <id>command with the following behavior

  • if the id does not exist a "no such image" error is displayed
  • if the id references multiple repositories a "App is referenced in multiple repositories" error is displayed
  • if only one App is referenced by this id then this App is removed from the store

- A picture of a cute animal (not mandatory)
i6W2VufoExaRznkM

Signed-off-by: Jean-Christophe Sirot <jean-christophe.sirot@docker.com>
internal/store/bundle.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Nov 13, 2019

Codecov Report

Merging #740 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #740   +/-   ##
=====================================
  Coverage      70%    70%           
=====================================
  Files          64     64           
  Lines        3600   3600           
=====================================
  Hits         2520   2520           
  Misses        757    757           
  Partials      323    323

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3328d80...26cc7d3. Read the comment docs.

Signed-off-by: Jean-Christophe Sirot <jean-christophe.sirot@docker.com>
if len(b.refsMap[id]) == 0 {
return fmt.Errorf("no such image %q", reference.FamiliarString(ref))
} else if len(b.refsMap[id]) > 1 {
return fmt.Errorf("unable to delete %q - App is referenced in multiple repositories", reference.FamiliarString(ref))
Copy link
Member

Choose a reason for hiding this comment

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

Should we tell the user which repositories?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the docker image rm command does not do it

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@silvin-lubecki silvin-lubecki merged commit 27a4e6e into docker:master Nov 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants