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

[ExtendedTime] More clean up #969

Merged
merged 12 commits into from
Oct 22, 2024
Merged

[ExtendedTime] More clean up #969

merged 12 commits into from
Oct 22, 2024

Conversation

Tang8330
Copy link
Contributor

@Tang8330 Tang8330 commented Oct 21, 2024

  • Addressing PR comments
  • Adding the concept of default layout
  • Removing NestedKind variables (that were seldomly used)

@Tang8330 Tang8330 changed the title [go] Use layout [ExtendedTime] Specify layout in more places Oct 21, 2024
@@ -11,18 +11,22 @@ import (

type Time struct{}

func (Time) ToKindDetails() typing.KindDetails {
return typing.NewExtendedTimeDetails(typing.ETime, ext.TimeKindType, "")
func (Time) layout() string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addresses #967 (comment)

@Tang8330 Tang8330 changed the title [ExtendedTime] Specify layout in more places [ExtendedTime] More clean up Oct 22, 2024
@Tang8330 Tang8330 marked this pull request as ready for review October 22, 2024 00:11
@Tang8330 Tang8330 requested a review from a team as a code owner October 22, 2024 00:11
func (e ExtendedTime) MarshalJSON() ([]byte, error) {
return json.Marshal(e.String(""))
return json.Marshal(e.ts)
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

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

@Tang8330 Tang8330 merged commit ae18c5c into master Oct 22, 2024
3 checks passed
@Tang8330 Tang8330 deleted the refactor-extended-time-part2 branch October 22, 2024 18:41
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.

2 participants