Updated developer doc to explain external CLI #35681

javabrett · 2017-12-02T08:25:35Z

Need some doc on the move of the CLI to docker-ce, the included/locked/static CLI and how to replace it.

AkihiroSuda · 2017-12-02T08:32:13Z

docs/contributing/set-up-dev-env.md

@@ -187,13 +187,53 @@ can take over 15 minutes to complete.
   hack/make.sh binary install-binary run
   ```

-9. Inside your container, check your Docker version.
+9. Inside your container, check your Docker versions by running both `docker --version` and `docker version`.


No need to run docker --version.
The client version is printed in docker version as well

thaJeztah

Thank you @javabrett! I left some comments inline

thaJeztah · 2017-12-05T03:00:21Z

docs/contributing/set-up-dev-env.md

   ```
+
+   Notice the split versions between client and server, which might be unexpected. In more recent times the Docker CLI component (which provides the `docker` command) has split out from the `moby` project and is now maintained in [docker/docker-ce](https://github.com/docker/docker-ce).  The `moby` project now defaults to a [fixed version](https://github.com/docker/docker-ce/commits/v17.06.0-ce) of the `docker` CLI for integration tests.


Can you

wrap this paragraph to 80-chars?

remove the back-ticks around moby, and use a capital (in this case it's describing the name of the project, not a command)

Done and done.

thaJeztah · 2017-12-05T03:01:36Z

docs/contributing/set-up-dev-env.md

+   DOCKER_CLI_PATH=/host/path/to/cli/binary make shell
+   then change the cli and compile into a binary at the same location.
+   ```
+   By setting `DOCKER_CLI_PATH` you can supply a newer `docker` CLI to the server development container for testing and for `integration-cli` test-execution:


Can you wrap this to 80-chars as well?

Perhaps we should mention that the binary needs to be built separately, and must be a Linux binary (as it's used inside the container)

Done and added a note about building that for Linux. Note that docker-ce doesn't have a lot of build-docs at the moment - not sure it's worth an issue since most folks will bumble their way through a make.

GordonTheTurtle · 2017-12-05T04:25:15Z

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "docs-contributing-2" git@github.com:javabrett/moby.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842357011432
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

javabrett · 2017-12-11T08:53:10Z

Anything else needed for this one?

thaJeztah

Sorry for the delay, I missed your previous comment; I left comments inline; can you also squash your commits (we don't need to preserve the review history as separate commits, just a single commit should be ok for this change) 👍

thaJeztah · 2017-12-12T02:43:25Z

docs/contributing/set-up-dev-env.md

+   # docker --version
+   Docker version 17.09.0-dev, build 
+   ```
+	 This Docker CLI should be build from the [docker-ce


Looks like you;

used a tab for indentation (but we standardise on space in this document)

also the subsequent lines are not indented

can you fix that here (and the other lines?); if it's helpful; here's the patch that you'd need;

diff --git a/docs/contributing/set-up-dev-env.md b/docs/contributing/set-up-dev-env.md index 9568b4ca8..94cc957be 100644 --- a/docs/contributing/set-up-dev-env.md +++ b/docs/contributing/set-up-dev-env.md @@ -206,14 +206,14 @@ can take over 15 minutes to complete. OS/Arch: linux/amd64 Experimental: false ``` - - Notice the split versions between client and server, which might be -unexpected. In more recent times the Docker CLI component (which provides the -`docker` command) has split out from the Moby project and is now maintained in -[docker/docker-ce](https://github.com/docker/docker-ce). The Moby project now -defaults to a [fixed -version](https://github.com/docker/docker-ce/commits/v17.06.0-ce) of the -`docker` CLI for integration tests. + + Notice the split versions between client and server, which might be + unexpected. In more recent times the Docker CLI component (which provides + the `docker` command) has split out from the Moby project and is now + maintained in [docker/docker-ce](https://github.com/docker/docker-ce). The + Moby project now defaults to a [fixed + version](https://github.com/docker/docker-ce/commits/v17.06.0-ce) of the + `docker` CLI for integration tests. You may have noticed the following message when starting the container with the `shell` command: @@ -222,10 +222,11 @@ version](https://github.com/docker/docker-ce/commits/v17.06.0-ce) of the DOCKER_CLI_PATH=/host/path/to/cli/binary make shell then change the cli and compile into a binary at the same location. ``` - By setting `DOCKER_CLI_PATH` you can supply a newer `docker` CLI to the -server development container for testing and for `integration-cli` -test-execution: - + + By setting `DOCKER_CLI_PATH` you can supply a newer `docker` CLI to the + server development container for testing and for `integration-cli` + test-execution: + ```none make DOCKER_CLI_PATH=/home/ubuntu/git/docker-ce/components/packaging/static/build/linux/docker/docker BIND_DIR=. shell ... @@ -234,9 +235,11 @@ test-execution: # docker --version Docker version 17.09.0-dev, build ``` - This Docker CLI should be build from the [docker-ce -project](https://github.com/docker/docker-ce) and needs to be a Linux binary. - + + This Docker CLI should be build from the [docker-ce + project](https://github.com/docker/docker-ce) and needs to be a Linux + binary. + Inside the container you are running a development version. This is the version on the current branch. It reflects the value of the `VERSION` file at the root of your `docker-fork` repository.

Thanks, done, and especially thanks for the patch 👍.

thaJeztah

LGTM, thanks!

thaJeztah · 2017-12-12T21:28:45Z

ping @AkihiroSuda PTAL

AkihiroSuda · 2017-12-13T00:48:08Z

docs/contributing/set-up-dev-env.md

+   Notice the split versions between client and server, which might be
+   unexpected. In more recent times the Docker CLI component (which provides
+   the `docker` command) has split out from the Moby project and is now
+   maintained in [docker/docker-ce](https://github.com/docker/docker-ce). The


Shouldnt we mention docker/cli repo?

oh, hm, yes, we probably should

was working on cherry-picks, so didn't even notice 😓

AkihiroSuda · 2017-12-13T03:01:25Z

docs/contributing/set-up-dev-env.md

+   Docker version 17.09.0-dev, build 
+   ```
+
+    This Docker CLI should be build from the [docker-ce


typo: build -> built

AkihiroSuda · 2017-12-13T03:01:37Z

LGTM except typo

…moby->docker-ce. Signed-off-by: Brett Randall <javabrett@gmail.com>

thaJeztah · 2017-12-13T08:01:02Z

Thanks!!

GordonTheTurtle added the status/0-triage label Dec 2, 2017

javabrett force-pushed the docs-contributing-2 branch 2 times, most recently from e1dadd4 to e1936dc Compare December 2, 2017 08:28

AkihiroSuda reviewed Dec 2, 2017

View reviewed changes

javabrett force-pushed the docs-contributing-2 branch from e1936dc to 1c9e9f4 Compare December 2, 2017 08:34

thaJeztah requested changes Dec 5, 2017

View reviewed changes

thaJeztah added area/project status/3-docs-review and removed status/0-triage labels Dec 5, 2017

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Dec 5, 2017

javabrett force-pushed the docs-contributing-2 branch from ec50f95 to fe4126f Compare December 5, 2017 04:29

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Dec 5, 2017

javabrett force-pushed the docs-contributing-2 branch from fe4126f to be0502b Compare December 7, 2017 03:14

thaJeztah requested changes Dec 12, 2017

View reviewed changes

javabrett force-pushed the docs-contributing-2 branch from be0502b to 40b61b8 Compare December 12, 2017 08:56

thaJeztah approved these changes Dec 12, 2017

View reviewed changes

AkihiroSuda reviewed Dec 13, 2017

View reviewed changes

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Dec 13, 2017

javabrett force-pushed the docs-contributing-2 branch from 056d984 to dbb84a4 Compare December 13, 2017 02:37

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Dec 13, 2017

AkihiroSuda reviewed Dec 13, 2017

View reviewed changes

Updated developer doc to explain included static cli and move of cli …

b710916

…moby->docker-ce. Signed-off-by: Brett Randall <javabrett@gmail.com>

javabrett force-pushed the docs-contributing-2 branch from dbb84a4 to b710916 Compare December 13, 2017 03:32

AkihiroSuda approved these changes Dec 13, 2017

View reviewed changes

AkihiroSuda merged commit b595858 into moby:master Dec 13, 2017

javabrett deleted the docs-contributing-2 branch December 13, 2017 04:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated developer doc to explain external CLI #35681

Updated developer doc to explain external CLI #35681

javabrett commented Dec 2, 2017

AkihiroSuda Dec 2, 2017

javabrett Dec 2, 2017

thaJeztah left a comment

thaJeztah Dec 5, 2017

javabrett Dec 5, 2017

thaJeztah Dec 5, 2017

javabrett Dec 5, 2017

GordonTheTurtle commented Dec 5, 2017

javabrett commented Dec 11, 2017

thaJeztah left a comment

thaJeztah Dec 12, 2017

javabrett Dec 12, 2017

thaJeztah left a comment

thaJeztah commented Dec 12, 2017

AkihiroSuda Dec 13, 2017

thaJeztah Dec 13, 2017

AkihiroSuda Dec 13, 2017

javabrett Dec 13, 2017

AkihiroSuda commented Dec 13, 2017

thaJeztah commented Dec 13, 2017

		```

		Notice the split versions between client and server, which might be unexpected. In more recent times the Docker CLI component (which provides the `docker` command) has split out from the `moby` project and is now maintained in [docker/docker-ce](https://github.com/docker/docker-ce). The `moby` project now defaults to a [fixed version](https://github.com/docker/docker-ce/commits/v17.06.0-ce) of the `docker` CLI for integration tests.

Updated developer doc to explain external CLI #35681

Updated developer doc to explain external CLI #35681

Conversation

javabrett commented Dec 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GordonTheTurtle commented Dec 5, 2017

javabrett commented Dec 11, 2017

thaJeztah left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah commented Dec 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkihiroSuda commented Dec 13, 2017

thaJeztah commented Dec 13, 2017