-
Notifications
You must be signed in to change notification settings - Fork 30
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
[ExtendedTime] More clean up #969
Conversation
Tang8330
commented
Oct 21, 2024
•
edited
Loading
edited
- Addressing PR comments
- Adding the concept of default layout
- Removing NestedKind variables (that were seldomly used)
@@ -11,18 +11,22 @@ import ( | |||
|
|||
type Time struct{} | |||
|
|||
func (Time) ToKindDetails() typing.KindDetails { | |||
return typing.NewExtendedTimeDetails(typing.ETime, ext.TimeKindType, "") | |||
func (Time) layout() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addresses #967 (comment)
lib/typing/ext/time.go
Outdated
func (e ExtendedTime) MarshalJSON() ([]byte, error) { | ||
return json.Marshal(e.String("")) | ||
return json.Marshal(e.ts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not taking the NestedKind's type and format into account?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah. I'm purposefully moving away from using the ExtendedTime's nested type and format into account.
My goal is to eventually make ExtendedTime just an alias for time.Time
.
MarshalJSON was created specifically for nested MongoDB objects and this code is consistent with how Mongo marshals Datetime https://github.com/mongodb/mongo-go-driver/blob/c91b204cc72025ea0c8d0845b84816b8c9e88903/bson/primitive.go#L59-L61
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, let me get it to look identical to MongoDB's