Skip to content
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

Add CreatedAtTimestamp and fix CreatedAt docs #4765

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

simonha9
Copy link

@simonha9 simonha9 commented Jan 9, 2024

Fixes #2413

- What I did
Added CreatedAtTimestamp and fix documentation for CreatedAt.

- How I did it
As above.

- How to verify it
TestContainerContextWriteJSON in container_test.go

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)
image

Signed-off-by: Simon H <simon9ha@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #4765 (bd36415) into master (cfe18f5) will decrease coverage by 0.01%.
Report is 41 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4765      +/-   ##
==========================================
- Coverage   59.62%   59.62%   -0.01%     
==========================================
  Files         287      285       -2     
  Lines       24777    24776       -1     
==========================================
- Hits        14774    14772       -2     
  Misses       9117     9117              
- Partials      886      887       +1     

@@ -192,11 +192,16 @@ func (c *ContainerContext) Command() string {
return strconv.Quote(command)
}

// CreatedAt returns the "Created" date/time of the container as a unix timestamp.
// CreatedAt returns the "Created" date/time of the container as a string
Copy link
Member

Choose a reason for hiding this comment

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

could we be more explicit as to which format the string would be in?
According to the docs this would be the format.
"2006-01-02 15:04:05.999999999 -0700 MST"

There's also a difference if c.c.Created contains nanoseconds.

withNanoseconds = 2000-02-01 12:13:14.000000015 +0000 UTC
withoutNanoseconds = 2000-02-01 12:13:14 +0000 UTC

https://pkg.go.dev/time#Time.String

Comment on lines +200 to +203
// CreatedAtTimestamp returns the "Created" date/time of the container as a unix timestamp
func (c *ContainerContext) CreatedAtTimestamp() time.Time {
return time.Unix(c.c.Created, 0)
}
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about this function here. It's not being used anywhere right now (except for tests). I would be in favor of just not adding this function since there is no use for it currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ContainerFormat's CreatedAt has wrong documentation
3 participants