You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
NewDateTime is a representation of zero value for DateTime type
and IsZero says:
IsZero returns whether the date time is a zero value
Given these docs, I expected that NewDateTime() would match the zero value (e.g., var d DateTime), and that IsZero() would be true for it. However, that's not the case as NewDateTime() returns Unix(0, 0) which is different from a zero DateTime.
A couple of approaches to fix this:
Approach 1: return time.Time{} in NewDateTime
Rather than initializing using time.Unix(0, 0), NewDateTime could return time.Time{}, which would better align with the docs, as it suggests that NewDateTime() should match var d strfmt.DateTime which it doesn't currently.
However, this likely has a larger impact for users of NewDateTime since the value returned is changing.
IMO this is the more correct fix.
Approach 2: Update IsZero IsZero could be updated to check for IsUnixZero() in addition to checking for zero time.Time.
I would also recommend the documentation for NewDateTime is updated to indicate that it uses Unix(0, 0) rather than matching a zero DateTime.
The text was updated successfully, but these errors were encountered:
Repro:
Output:
Want:
The
NewDateTime
documentation says:and
IsZero
says:Given these docs, I expected that
NewDateTime()
would match the zero value (e.g.,var d DateTime
), and thatIsZero()
would be true for it. However, that's not the case asNewDateTime()
returnsUnix(0, 0)
which is different from a zeroDateTime
.A couple of approaches to fix this:
Approach 1: return
time.Time{}
inNewDateTime
Rather than initializing using
time.Unix(0, 0)
,NewDateTime
could returntime.Time{}
, which would better align with the docs, as it suggests thatNewDateTime()
should matchvar d strfmt.DateTime
which it doesn't currently.However, this likely has a larger impact for users of
NewDateTime
since the value returned is changing.IMO this is the more correct fix.
Approach 2: Update
IsZero
IsZero
could be updated to check forIsUnixZero()
in addition to checking for zerotime.Time
.I would also recommend the documentation for
NewDateTime
is updated to indicate that it usesUnix(0, 0)
rather than matching a zeroDateTime
.The text was updated successfully, but these errors were encountered: