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
Adding clarification to save/load and export/import commands #550
Conversation
Closes: moby/moby#34926 Signed-off-by: Serguei Bezverkhi <sbezverk@cisco.com>
Codecov Report
@@ Coverage Diff @@
## master #550 +/- ##
==========================================
+ Coverage 49.08% 49.09% +<.01%
==========================================
Files 200 200
Lines 16447 16451 +4
==========================================
+ Hits 8073 8076 +3
- Misses 7954 7955 +1
Partials 420 420 |
/assign @thaJeztah |
Args: cli.RequiresMinArgs(1), | ||
Use: "import [OPTIONS] file|URL|- [REPOSITORY[:TAG]]", | ||
Short: `Import the contents from a tar archive to create a filesystem image | ||
previously exported by "docker export" command.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking of this now, I think docker import
can be used to just import a tar-bal rootfs for the container (which is not created by docker export
), so we may need some more working here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @mstanleyjones
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying that what ever docker export create is NOT what is used for "docker import" input? Man if it is the case things are even more obscure than I thought. What then creates tar archive for "docker import"?
docker export --help
Usage: docker export [OPTIONS] CONTAINER
Export a container's filesystem as a tar archive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any tar
command or tar library could create it, it's not just docker export
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thaJeztah Do you want me to ad a couple of words here ? Something like "exported by docker export
or tar
commands"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm -1 on this change. Short descriptions should be a single line so they display properly in help.
If this needs to be clarified, let's do that in the reference docs under docs/
Args: cli.ExactArgs(1), | ||
Use: "export [OPTIONS] CONTAINER", | ||
Short: `Export a container's filesystem as a tar archive | ||
"docker import" must be used to restore the filesystem from the exported tar archive.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Short
should be a line short line, so i don't think this is the appropriate place to document this.
@dnephin curious how often you open a doc instead of running "command help"? From convenience point of view this approach is way more useful, surprised to see -1 to something as obvious as this. |
How often do you need to be reminded that import and export are complimentary operations? Probably only once. But I might run If we think we need the full reference docs displayed as part of |
@dnephin if you check other commands, example "kubectl --help" you will see that it generates around 1 page of output. So it is not something never been done or bad thing to do, it is very subjective opinion. I think the key of everything is its usability and adding this extra line makes things a bit more clear without going through man pages. |
@sbezverk Thank you for taking the time to open a PR. I'm sorry that this did not get the necessary attention at the time to get merged. I'll be closing this since it's been quite a while since this PR has been created and a lot has changed since then. Feel free to open another PR if this is still a relevant topic you wish to contribute! |
Closes: moby/moby#34926