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.

Sign up for GitHub

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

Jump to bottom

Add --prune to stack deploy #31302

Merged
merged 1 commit into from
Mar 15, 2017
Merged

Add --prune to stack deploy #31302

merged 1 commit into from
Mar 15, 2017

Conversation

dnephin
Copy link
Member

@dnephin dnephin commented Feb 23, 2017 •
edited by thaJeztah
Loading

Fixes #29898

cc @vdemeester

Description for changelog

+ Add `--prune` option to `docker stack deploy` [#31302](https://github.com/docker/docker/pull/31302)

vdemeester
vdemeester reviewed Feb 23, 2017
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

Desgin LGTM.
Code looks good, gotta try this out tomorrow but 👍

cli/internal/test/cli.go Outdated
@@ -42,7 +48,12 @@ func (c *FakeCli) Out() *command.OutStream {
return command.NewOutStream(c.out)
}

// In returns thi input stream the cli will use
Copy link
Member

Choose a reason for hiding this comment

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

😂

@thaJeztah
Copy link
Member

Should we have a --remove-orphans alias to make adoption easier for those that are used to docker-compose?

@vieux
Copy link
Contributor

vieux commented Feb 23, 2017

should docker system prune prune stacks as well ?

@thaJeztah
Copy link
Member

Perhaps documenting it somewhere (some sort of migration guide) could work

@dnephin
Copy link
Member Author

dnephin commented Mar 3, 2017

Should we have a --remove-orphans alias to make adoption easier for those that are used to docker-compose?

I think there are enough CLI differences already that no one should expect the same flags.

should docker system prune prune stacks as well ?

There is no object on the server yet, so I don't think so. We don't have prune for services either, right?

@cpuguy83
Copy link
Member

cpuguy83 commented Mar 7, 2017

Design OK.

@dnephin
Copy link
Member Author

dnephin commented Mar 7, 2017

Moved to code review

cpuguy83
cpuguy83 approved these changes Mar 7, 2017
View reviewed changes
Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

vdemeester
vdemeester approved these changes Mar 12, 2017
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐸

@vdemeester
Copy link
Member

/cc @thaJeztah for docs review

@thaJeztah
Copy link
Member

@dnephin can you update the reference docs for the new flag?

This also needs an update to the bash/zsh completion scripts

@dnephin
Copy link
Member Author

dnephin commented Mar 13, 2017

I think this part of the command line reference is generated now, but I've added them anyway.

thaJeztah
thaJeztah approved these changes Mar 14, 2017
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah
Copy link
Member

may need to be squashed though 😇

@dnephin
Add --prune to stack deploy.
644fd80 
Add to command line reference.

Signed-off-by: Daniel Nephin <dnephin@docker.com>
@dnephin
Copy link
Member Author

dnephin commented Mar 14, 2017

rebased

@thaJeztah
Copy link
Member

All green 👍

@thaJeztah thaJeztah merged commit b0d1936 into moby:master Mar 15, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.04.0 milestone Mar 15, 2017
@thaJeztah thaJeztah modified the milestones: 17.05.0, 17.04.0 Mar 15, 2017
@dnephin dnephin deleted the purge-orphaned-services branch March 15, 2017 14:31
@thaJeztah thaJeztah mentioned this pull request Mar 22, 2017
Open
dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017
@thaJeztah
Merge pull request moby#31302 from dnephin/purge-orphaned-services
69b7bf5 
Add --prune to stack deploy
@albers albers mentioned this pull request May 8, 2017
Merged
@lukewpatterson lukewpatterson mentioned this pull request Jun 14, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdemeester vdemeester vdemeester approved these changes

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

@tonistiigi tonistiigi

Labels
impact/changelog impact/cli impact/documentation status/4-merge
Projects
None yet
Milestone
17.05.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@dnephin @thaJeztah @vieux @cpuguy83 @vdemeester @tonistiigi @GordonTheTurtle