30431 implement format for history with docs #30962
Conversation
cli/command/formatter/history.go
Outdated
There was a problem hiding this comment.
Please add MarshalJSON so that {{json .}} works
There was a problem hiding this comment.
The granularity of the added tests seems different from existing tests.
(Please refer to existing *_test.go)
There was a problem hiding this comment.
I added a test against the full table view, but I'm not sure if this is what you meant.
|
Sorry, I was having trouble running the integration tests, I'll fix this up tonight and update the PR. |
fb537b9 to
6b40c3c
Compare
2438070 to
9523ac5
Compare
|
Is this still failing CI? |
|
I confirmed it works, so code LGTM
|
39ea636 to
3be6e78
Compare
|
Squashed and removed the integration test. |
|
@yongtang PTAL? |
There was a problem hiding this comment.
It seems both Created and CreatedSince has been defined for placeholders. Though here the Created is not mentioned in the docs.
I am wondering if we add both in the docs, or, maybe we could just merge Created and CreatedSince into one placeholder func?
|
@TheHipbot it looks like You may need to check the value of Also, I am wondering if it make sense to merge |
|
@yongtang I was originally thinking to provide both options, which I should have added to the docs. That being said it also makes sense to combine so I will update the PR with the two combined as |
3be6e78 to
49c9964
Compare
49c9964 to
ad95a3b
Compare
39ff38d to
60cbecd
Compare
|
@yongtang PTAL |
There was a problem hiding this comment.
Can you update the doc to align the table like below so that it is easy to update in the future?
| Placeholder | Description |
| --------------- | ----------- |
| `.ID` | Image ID |
| `.CreatedSince` | Elapsed time since the image was created if --human=true, otherwise timestamp of when image was created |
| `.CreatedAt` | Timestamp of when image was created |
| `.CreatedBy` | Command that was used to create the image |
| `.Size` | Image disk size |
| `.Comment` | Comment for image |
cli/command/formatter/history.go
Outdated
There was a problem hiding this comment.
Also, it seems that CreatedAt and CreatedSince is pretty much the same except for the ago. And --human flag does not have an impact to the rendering
I am wondering if CreatedAt should use time.Unix(int64(c.h.Created), 0).String() (just like docker images --format) instead?
There was a problem hiding this comment.
I ended up having to separate these again due to some changes in how the headers are output with the formatters. I added to the docs to have both in the template.
…ormatter
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Adds to history documentation for --format
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Adds MarshalJSON to historyContext for {{json .}} format
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Adds back the --human option to history command
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Cleans up formatter around --human option for history, Adds integration test for --format option of history
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Adds test for history formatter checking full table results, Runs go fmt on touched files
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Fixes lint errors in formatter/history
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
Runs go fmt on cli/command/formatter/history.go
Signed-off-by: Jeremy Chambers <jeremy@thehipbot.com>
sRemoves integration test for --format option of history
Merges Created and CreatedSince in docker history formatter, Updates docs and tests
60cbecd to
6bc1f34
Compare
|
Looks like Jenkins ran out of space on one of those builds |
There was a problem hiding this comment.
LGTM 🐮
/cc @thaJeztah for docs review
|
ping @thaJeztah |
There was a problem hiding this comment.
LGTM
I agree with #30962 (comment), but it's just a nit so let's get this merged. I can do a quick follow up to address it
…or-history-with-docs 30431 implement format for history with docs
- What I did
Adds the
--formatoption to the history command. Part of #30431- How I did it
Refactored
historyto use a formatter, add the format flag- How to verify it
Wrote unit tests to check that formatter is working correctly.
- Description for the changelog
Adds
--formatoption tohistorycommand- A picture of a cute animal (not mandatory but encouraged)