Add pointer conversion utilities to transform int64 to time.Time #1433
Conversation
aws/convert_types.go
Outdated
| @@ -311,6 +311,15 @@ func TimeValue(v *time.Time) time.Time { | |||
| return time.Time{} | |||
| } | |||
|
|
|||
| // Int64TimeValue converts an int64 pointer to a time.Time value or | |||
| // time.Time{} if the pointer is nil. | |||
| func Int64TimeValue(v *int64) time.Time { | |||
There was a problem hiding this comment.
It would be helpful for this to specify the resolution the int64is expected to be. In this case it looks to be millisecond -> second. Is this correct?
Alternatively dropping the Int64 part all together and just SecondsTimeValue and MillisecondTimeValue with int64 input param. Bit more verbose, but clear on the expectations.
There was a problem hiding this comment.
Another approach could be to extract the highest time resolution possible (millis) and keep the function name Int64TimeValue. Rounding the output to seconds feels like something that the SDK should not be directly responsible for. Thoughts? I'm open to either approach as I'm not sure which follows convention more closely here.
There was a problem hiding this comment.
Since i don't think there is any requirement required for services to return a specific resolution of time when used in number format I think having a set expectation would be the best approach.
Milliseconds would provide the most general usage. The downside to this would be that i think it would defeat the purpose of this utility for second based int64 fields as the user would need to dereference the *int64 value shift for milliseconds, and grab the pointer again before using the utility. This is where the idea of resolution specific methods came from.
There was a problem hiding this comment.
Understood and agreed. Let me get that up for us.
See: aws#1432 for discussion around adding this feature.
TylerBrock
force-pushed
the
add-int64-time-value
branch
from
6b89d0b to
d7eb80f
Compare
TylerBrock
changed the title
|
Thanks for updating the names. Would you mind adding a test case for each helper utility to the In addition one minor tweak to the wording of the functions' doc comment would be to include that seconds and milliseconds are since |
TylerBrock
force-pushed
the
add-int64-time-value
branch
from
a5aa894 to
7ffe2b2
Compare
See: #1432 for discussion around adding this feature.