Skip to content

Add missing mandatory arguments.#1986

Closed
JerryChin wants to merge 1 commit into
docker:masterfrom
JerryChin:patch-2
Closed

Add missing mandatory arguments.#1986
JerryChin wants to merge 1 commit into
docker:masterfrom
JerryChin:patch-2

Conversation

@JerryChin

Copy link
Copy Markdown

Proposed changes

Reason
The command given herein is incomplete, and it will lead to an erroneous outcome, which discourages the learners badly.

Action
I completed the command with the proper option and argument.

Unreleased project version (optional)

N/A

Related issues (optional)

N/A

The command given herein is incomplete, and it will lead to an erroneous outcome,  which discourages the learners badly.
@mdlinville

Copy link
Copy Markdown

@londoncalling PTAL.I think if your compose file is docker-compose.yml, you don't have to specify the -c option.

@mdlinville mdlinville requested a review from londoncalling March 1, 2017 22:40
@londoncalling

londoncalling commented Mar 2, 2017

Copy link
Copy Markdown
Contributor

@JerryChin , @mstanleyjones the command is correct, as written in the appropriate section of the tutorial.

The page you updated to include -c is just an intro to the command, it doesn't show the full command (e.g., doesn't mention the options --compose-file | -c or app name). If you are following along with the tutorial, you would not have the .yml in place, nor any Docker machines or a swarm set up and running. So I don't want to give the full command for someone to try at this point, it would be misleading, and wouldn't work anyway. What I can do is link to the command syntax here and in the relevant steps, so that it's clear we are not suggesting to run the command at this point.

The steps in the next topics (after you've done the set up and gotten the swarm running) show the full format of the command: Deploy the app

docker stack deploy --compose-file docker-stack.yml vote

(we use the long form, --compose-file instead of -c)

@ManoMarks any other thoughts or do you want to jump in?

@londoncalling

londoncalling commented Mar 2, 2017

Copy link
Copy Markdown
Contributor

@JerryChin I'm going to close this PR when PR #2042 is merged. That should address your feedback more thoroughly and in line with the way the tutorial is written. Thanks!

@JerryChin

JerryChin commented Mar 2, 2017 via email

Copy link
Copy Markdown
Author

@londoncalling

Copy link
Copy Markdown
Contributor

@JerryChin Cool, thanks.

The updates from PR #2042 are published. The topics I updated per your feedback are:

Docker stacks and services

Deploy the app

I'm closing this PR. Please send more comments if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants