Skip to content

Migrate cp command to cobra#23348

Merged
LK4D4 merged 1 commit intomoby:masterfrom
vdemeester:migrate-cp-to-cobra
Jun 14, 2016
Merged

Migrate cp command to cobra#23348
LK4D4 merged 1 commit intomoby:masterfrom
vdemeester:migrate-cp-to-cobra

Conversation

@vdemeester
Copy link
Copy Markdown
Member

Moves the cp command to api/client/container/cp.go and use cobra 🐍.

Something a little bit weird about cp though is that there is two "Use"…

Usage:  docker cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH|-
        docker cp [OPTIONS] SRC_PATH|- CONTAINER:DEST_PATH

Copy files/folders between a container and the local filesystem
Use '-' as the source to read a tar archive from stdin
and extract it to a directory destination in a container.
Use '-' as the destination to stream a tar archive of a
container source to stdout.

… so I did a little weird thingie but it would be awesome if cobra would support it 👼

/cc @dnephin @thaJeztah @LK4D4 @cpuguy83

🐸

Signed-off-by: Vincent Demeester [email protected]

Comment thread api/client/container/cp.go Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should be in Long, and Short should be the one line

"Copy files/folders between a container and the local filesystem"

Copy link
Copy Markdown
Member Author

@vdemeester vdemeester Jun 14, 2016

Choose a reason for hiding this comment

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

I tend to agree with you but this breaks current output.

before:

λ docker cp
docker: "cp" requires 2 arguments.
See 'docker cp --help'.

Usage:  docker cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH|-
        docker cp [OPTIONS] SRC_PATH|- CONTAINER:DEST_PATH

Copy files/folders between a container and the local filesystem
Use '-' as the source to read a tar archive from stdin
and extract it to a directory destination in a container.
Use '-' as the destination to stream a tar archive of a
container source to stdout

after:

λ ./bundles/latest/dynbinary-client/docker cp
"docker cp" requires exactly 2 argument(s).
See 'docker cp --help'.

Usage:  docker cp [OPTIONS] CONTAINER:SRC_PATH DEST_PATH|-
        docker cp [OPTIONS] SRC_PATH|- CONTAINER:DEST_PATH

Copy files/folders between a container and the local filesystem

docker cp --help will be the same.

@thaJeztah wdyt ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

sgtm yes

@vdemeester vdemeester force-pushed the migrate-cp-to-cobra branch from d2aae87 to bcc0d6d Compare June 10, 2016 10:08
@icecrime icecrime modified the milestone: 1.12.0 Jun 13, 2016
@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Jun 13, 2016

@vdemeester psst

Signed-off-by: Vincent Demeester <[email protected]>
@vdemeester vdemeester force-pushed the migrate-cp-to-cobra branch from bcc0d6d to 2e6db51 Compare June 14, 2016 15:17
@vdemeester
Copy link
Copy Markdown
Member Author

Rebased and updated 😉

@dnephin
Copy link
Copy Markdown
Member

dnephin commented Jun 14, 2016

LGTM

@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Jun 14, 2016

Ok, experimental hangs after integration tests pass.
LGTM

@LK4D4 LK4D4 merged commit 81a85cf into moby:master Jun 14, 2016
@vdemeester vdemeester deleted the migrate-cp-to-cobra branch June 14, 2016 19:30
@dnephin dnephin mentioned this pull request Jul 18, 2016
43 tasks
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.

6 participants