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

Allow rm of multiple apps in one command #775

Merged
merged 2 commits into from
Dec 2, 2019

Conversation

zappy-shu
Copy link
Contributor

- What I did

Adds ability to remove multiple running apps with a single command. E.g.:
docker app rm app1 app2

Attempts to remove all listed apps before returning any errors same as with docker app image rm.

- How I did it

Changed the ExactArgs to RequiredMinArgs and loop through the args, appending any errors which are all returned at the end if any exist.

- How to verify it

docker app run 2 apps with names app1 and app2 then docker app rm app1 app2. The output should show each app being removed

- Description for the changelog

Allow removing of multiple Apps in one command using space delimited App names. E.g. docker app rm my-app-1 my-app-2

- A picture of a cute animal (not mandatory)

hippo-2019-12-02

@zappy-shu zappy-shu force-pushed the APP-308-rm-multi-apps branch 3 times, most recently from 9e894bb to 3453532 Compare December 2, 2019 13:03
Adds ability to remove multiple running apps with a single command. E.g.:
	docker app rm app1 app2
Attempts to remove all listed apps before returning any errors same as with docker app image rm.

Signed-off-by: Nick Adcock <nick.adcock@docker.com>
Copy link
Contributor

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

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

minor suggestion

internal/commands/remove.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 2, 2019

Codecov Report

Merging #775 into master will decrease coverage by 0.02%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #775      +/-   ##
==========================================
- Coverage   70.44%   70.41%   -0.03%     
==========================================
  Files          67       67              
  Lines        3729     3732       +3     
==========================================
+ Hits         2627     2628       +1     
- Misses        758      759       +1     
- Partials      344      345       +1
Impacted Files Coverage Δ
internal/commands/remove.go 50% <60%> (-0.95%) ⬇️

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 d119541...518754b. Read the comment docs.

Use hashicorp/go-multierror to handle multiple errors when removing multiple apps at once

Signed-off-by: Nick Adcock <nick.adcock@docker.com>
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

@zappy-shu zappy-shu merged commit b99a484 into docker:master Dec 2, 2019
@zappy-shu zappy-shu changed the title [WIP] Allow rm of multiple apps in one command Allow rm of multiple apps in one command Dec 2, 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

4 participants