Add `--filter scope=swarm|local` for `docker network ls` by yongtang · Pull Request #31529 · moby/moby

yongtang · 2017-03-03T19:06:28Z

- What I did

This fix tries to address the request in #31324 by adding --filter scope=swarm|local for docker network ls.

As docker network ls has a SCOPE column by default, it is natural to add the support of --filter scope=swarm|local.

- How I did it

This fix adds the scope=swarm|local support for docker network ls --filter.

Related docs has been updated.

- How to verify it

Additional unit test cases have been added.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

This fix fixes #31324.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

allencloud · 2017-03-05T02:30:23Z

I think we still need to update swagger.yml. WDYT? @yongtang

yongtang · 2017-03-05T18:39:32Z

Thanks @allencloud. The PR has been updated with changes in swagger.yaml added.

yongtang · 2017-03-05T18:44:10Z

@allencloud The PR has been updated. Thanks.

allencloud · 2017-03-06T02:14:36Z

api/swagger.yaml

I prefer to add scope before type since out of alphabetical order, WDYT?

yongtang · 2017-03-06T04:52:38Z

Thanks @allencloud. The swagger.yaml, docs, and man have been updated.

vdemeester

design LGTM

allencloud · 2017-03-07T02:26:01Z

docs/reference/commandline/network_ls.md

I think this should be local scope. ^^

@allencloud My bad. The PR has been updated. Thanks.

allencloud · 2017-03-07T02:26:25Z

man/src/network/ls.md

I think this should be local scope. ^^

Done. Thanks.

thaJeztah · 2017-03-22T15:01:02Z

Design, and code looks good to me, but needs a rebase @yongtang 😅

thaJeztah · 2017-03-22T15:02:35Z

Don't forget to add a mention to the API history; https://github.com/docker/docker/blob/master/docs/api/version-history.md

cpuguy83 · 2017-03-22T15:03:49Z

api/swagger.yaml

There is also global I think?

hm, is there 🤔

Thanks @cpuguy83 @thaJeztah. Yes I forgot libnetwork have the legacy external K-V store mode (global).

The PR has been updated and rebased. Please take a look.

cpuguy83

LGTM

thaJeztah · 2017-03-23T22:22:48Z

docs/api/version-history.md

I think this should go under API 1.29 now - we had to bump the API version for 17.03.1

Thanks @thaJeztah. The PR has been updated with version changed.

This fix tries to address the request in 31324 by adding `--filter scope=swarm|local` for `docker network ls`. As `docker network ls` has a `SCOPE` column by default, it is natural to add the support of `--filter scope=swarm|local`. This fix adds the `scope=swarm|local` support for `docker network ls --filter`. Related docs has been updated. Additional unit test cases have been added. This fix fixes 31324. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

vdemeester

LGTM 🐮
/cc @thaJeztah for docs-review 👼

thaJeztah

LGTM! thanks!

thaJeztah · 2017-03-24T11:23:49Z

windows completed but failed to notify github

java.net.SocketTimeoutException: connect timed out
Finished: SUCCESS

albers · 2017-03-24T12:08:25Z

Thanks @yongtang!

GordonTheTurtle added the status/0-triage label Mar 3, 2017

yongtang mentioned this pull request Mar 3, 2017

[Feature request] docker network ls --filter scope=swarm|local #31324

Closed

yongtang force-pushed the 31324-network-ls-filter-scope branch from a6fa0ea to 7744fbb Compare March 5, 2017 18:38

allencloud reviewed Mar 6, 2017

View reviewed changes

api/swagger.yaml Outdated

Copy link

Contributor

allencloud Mar 6, 2017

Choose a reason for hiding this comment

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

I prefer to add scope before type since out of alphabetical order, WDYT?

yongtang force-pushed the 31324-network-ls-filter-scope branch from 7744fbb to 49753b7 Compare March 6, 2017 04:52

vdemeester added status/1-design-review and removed status/0-triage labels Mar 6, 2017

vdemeester reviewed Mar 6, 2017

View reviewed changes

allencloud reviewed Mar 7, 2017

View reviewed changes

yongtang force-pushed the 31324-network-ls-filter-scope branch from 49753b7 to 46593c3 Compare March 7, 2017 02:36

GordonTheTurtle assigned duglin Mar 18, 2017

thaJeztah added status/2-code-review and removed status/1-design-review labels Mar 22, 2017

thaJeztah added impact/changelog impact/cli labels Mar 22, 2017

cpuguy83 reviewed Mar 22, 2017

View reviewed changes

yongtang force-pushed the 31324-network-ls-filter-scope branch from 46593c3 to d93a710 Compare March 23, 2017 16:41

cpuguy83 added the rebuild/janky label Mar 23, 2017

GordonTheTurtle removed the rebuild/janky label Mar 23, 2017

cpuguy83 approved these changes Mar 23, 2017

View reviewed changes

thaJeztah reviewed Mar 23, 2017

View reviewed changes

yongtang force-pushed the 31324-network-ls-filter-scope branch from d93a710 to 704ea8f Compare March 24, 2017 01:49

vdemeester approved these changes Mar 24, 2017

View reviewed changes

vdemeester added status/3-docs-review and removed status/2-code-review labels Mar 24, 2017

thaJeztah approved these changes Mar 24, 2017

View reviewed changes

thaJeztah merged commit 4e290f7 into moby:master Mar 24, 2017

GordonTheTurtle added this to the 17.05.0 milestone Mar 24, 2017

albers mentioned this pull request Mar 24, 2017

Add bash completion for network ls --filter scope #32081

Merged

yongtang deleted the 31324-network-ls-filter-scope branch March 24, 2017 15:48

sdurrheimer mentioned this pull request Mar 26, 2017

Add zsh completion for 'docker network ls --filter scope' #32114

Merged

Conversation

yongtang commented Mar 3, 2017

Uh oh!

allencloud commented Mar 5, 2017

Uh oh!

yongtang commented Mar 5, 2017

Uh oh!

yongtang commented Mar 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongtang commented Mar 6, 2017

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Mar 22, 2017

Uh oh!

thaJeztah commented Mar 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Mar 24, 2017

Uh oh!

albers commented Mar 24, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants