stage0: implement image list --full flag #1455
Conversation
jonboulle
changed the title
|
This documentation should be updated: https://github.com/coreos/rkt/blob/master/Documentation/subcommands/image.md |
|
Cool! A couple of things:
|
I'm quite worried about removing this as it's required in all the manifest and this will create some confusion. IMHO it looks cleaner to know the hash type. Additionally if the spec adds additional hash types the prefix will be needed and coming back to it will create some UX problems.
If keeping |
blixtra
force-pushed
the
blixtra/rkt-implement-image-list-full-flag
branch
from
a21165d
to
4a7ab0d
Compare
|
@sgotti Thanks for the input. I've updated the PR and with the hash identifier prefix the other commands do work fine. |
|
The code LGTM. @yifan-gu : do you know if there is any script parsing the output of TODO:
|
|
@alban Working on the tests and thanks fro the documentation heads-up. Should we really be parsing the command line output? Perhaps we could just have a json output format. I recall there being a ticket about that but I didn't find it with a quick search. |
I think we should not parse the output, but I wanted to check just in case some software does it anyway. |
|
@yifan-gu I took a quick look at that code and it looks like it should work with this change if you don't require the full hash. The output with that command is now simply... |
|
@blixtra I think this won't break the code until we have a collision on the shortened hash :) |
blixtra
force-pushed
the
blixtra/rkt-implement-image-list-full-flag
branch
from
cef7359
to
827e169
Compare
|
Added functional tests and the docs are updated. |
| // Run tests | ||
| for _, tt := range tests { | ||
| runCmd := fmt.Sprintf("%s %s", ctx.cmd(), tt.cmd) | ||
| t.Logf("executing: %v", runCmd) |
There was a problem hiding this comment.
It's useful to know the test number, can you add it?
blixtra
force-pushed
the
blixtra/rkt-implement-image-list-full-flag
branch
from
827e169
to
33668ea
Compare
|
Thanks for the feedback. Here's another go. |
blixtra
force-pushed
the
blixtra/rkt-implement-image-list-full-flag
branch
from
33668ea
to
2164938
Compare
|
LGTM when the Semaphore tests pass. Note: |
The output of rkt image list was not very pleasant due to the very long hash and data format. This change shortens the hash to show the hash identifier plus the first 12 alphanumeric digits after the '-' char by default. The date format is changed to use a human readable format such as "5 days ago". If one care to have the older output format, she can use the newly introduced --full flag. Similar to the --full flag in rkt list, this provides a full hash and date output string. A follow up change needs to be made so that the commands that take the image hash as an argument can use the short form. Currently the short version of the hash is of little use other than viewing. Closes rkt#1438 Closes rkt#1439
This commit refactors the functional call, panic pattern that was reused often in the functional tests.
The tests checks that `rkt image list` is returning the correct short hash and the commands accept image hashes as parameters work with that hash.
blixtra
force-pushed
the
blixtra/rkt-implement-image-list-full-flag
branch
from
2164938
to
4d23248
Compare
…t-full-flag stage0: implement image list --full flag
The output of rkt image list was not very pleasant due to the very
long hash and data format. This change shortens the hash to use the
first 12 alphanumeric digits after the '-' char. The date format is
changed to use a human readable format such as "5 days ago". For this, a new dependency was added, github.com/dustin/go-humanize.
If one care to have the older output format, she can use the newly
introduced --full flag. Similar to the --full flag in rkt list, this
provides a full hash and date output string.
A follow up change needs to be made so that the commands that take the
image hash as an argument can use the short form. Currently the short
version of the hash is of little use other than viewing