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

Adds IsZero functionality to DateTime #105

Merged
merged 2 commits into from Mar 16, 2023

Conversation

bg451
Copy link
Contributor

@bg451 bg451 commented Mar 2, 2023

This PR adds a new function IsZero to the DateTime struct. The goal of this is to satisfy a common interface, IsZeroer, used by go-yaml/yaml marshaller. IsZero already exists on the time.Time object, so this does not introduce any novel implementation.

For reference, here is the documentation
note
on using IsZero by the (yaml).Marshal function. If you have a struct generated by swagger that use a proto timestamp, a strfmt.DateTime is used. When you try to marshal this to yaml, all DateTime fields are skipped and excluded.

@bg451
Copy link
Contributor Author

bg451 commented Mar 2, 2023

cc @casualjim

@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Merging #105 (397e96b) into master (a239ff1) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
+ Coverage   81.52%   81.56%   +0.03%     
==========================================
  Files          12       12              
  Lines        2019     2023       +4     
==========================================
+ Hits         1646     1650       +4     
  Misses        295      295              
  Partials       78       78              
Impacted Files Coverage Δ
time.go 90.97% <100.00%> (+0.25%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@casualjim
Copy link
Member

This could also use a companion method that considers time.Unix(0,0) to be zero. Go's idea of a mindate is different from other languages.

This seems to be the pattern for it: golang/go#33597 (comment)

@bg451
Copy link
Contributor Author

bg451 commented Mar 13, 2023

This could also use a companion method that considers time.Unix(0,0) to be zero.

@casualjim Hm so turns out this behavior isn't as simple as you'd hope. The equivalency is timezone dependent, e.g. time.Unix(0, 0) != time.Unix(0, 0).UTC() on my laptop. When the locale is UTC, the equivalency check is fine, but this seems pretty prone to cause bugs to due the nuance of it.

@casualjim
Copy link
Member

We can do something like a package level method that captures the IsZero predicate. Similar to these vars: https://github.com/go-openapi/strfmt/blob/master/time.go#L83-L88

I think this should be configurable because if you are dealing with javascript or java clients and they send a min date, they should be able to determine what is the Zero date for their deployment

This PR adds a new function `IsZero` to the DateTime struct. The goal of
this is to satisfy a common interface, IsZeroer, used by go-yaml/yaml
marshaller. `IsZero` already exists on the time.Time object, so this
does not introduce any novel implementation.

For reference, here is the [documentation
note](https://pkg.go.dev/gopkg.in/yaml.v3#Marshal) on using IsZero by the
`(yaml).Marshal` function.

Signed-off-by: Brandon Gonzalez <bg@chronosphere.io>
Signed-off-by: Brandon Gonzalez <bg@chronosphere.io>
@bg451
Copy link
Contributor Author

bg451 commented Mar 14, 2023

@casualjim alrighty, updated

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.

None yet

2 participants