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.

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

Orchestrator column #973

Merged
merged 1 commit into from
Apr 30, 2018
Merged

Conversation

silvin-lubecki
Copy link
Contributor

- What I did
To perform administration tasks more easily, I added an ORCHESTRATOR column in the docker stack ls command.

⚠️ Depends on #903 ⚠️

- How I did it
I added a column in the Stack formatter, and hardcoded the orchestrator in each list command implementation.

- How to verify it

$ DOCKER_ORCHESTRATOR=kubernetes docker stack ls
NAME                SERVICES            ORCHESTRATOR
mystack             3                   Kubernetes

$ DOCKER_ORCHESTRATOR=swarm docker stack ls
NAME                SERVICES            ORCHESTRATOR
mystack             4                   Swarm

- Description for the changelog
Added ORCHESTRATOR column to docker stack ls command

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

@vdemeester
Copy link
Collaborator

Seems like you have test failure 😓

@silvin-lubecki
Copy link
Contributor Author

Don't know what you are talking about 😅

Copy link
Collaborator

@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 on commit 8108934

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.

left one comment for documentation, but otherwise looks good.

I added [WIP] to the PR title, as this depends on the other PR's being merged first

"Services": stackServicesHeader,
"Name": nameHeader,
"Services": stackServicesHeader,
"Orchestrator": stackOrchestrastorHeader,
Copy link
Member

Choose a reason for hiding this comment

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

Can you:

Copy link
Member

Choose a reason for hiding this comment

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

This should be fixed, can you PTAL? Thanks!

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 after the other PR's are merged and this is rebased

@mat007
Copy link
Member

mat007 commented Apr 30, 2018

Rebased (and squashed the doc update) !

@thaJeztah thaJeztah changed the title [WIP] Orchestrator column Orchestrator column Apr 30, 2018
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 👍

@thaJeztah thaJeztah merged commit 3a55cd7 into docker:master Apr 30, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.05.0 milestone Apr 30, 2018
@thaJeztah thaJeztah modified the milestones: 18.05.0, 18.06.0 Apr 30, 2018
@silvin-lubecki silvin-lubecki deleted the orchestrator-column branch May 18, 2018 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants