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

Implement MarshalJSON for query results #114

Merged
merged 4 commits into from
Feb 9, 2019

Conversation

xichen2020
Copy link
Owner

@xichen2020 xichen2020 commented Feb 8, 2019

cc @notbdu @black-adder

This PR implements MarshalJSON for the various types of query results.

Fixes #112

Copy link
Collaborator

@notbdu notbdu left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -16,18 +18,40 @@ const (
StringType
)

var (
nullBytes = []byte("null")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to define this somewhere in a shared location?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hm a bit overkill given this is only used in two places in two separate packages.

@@ -5,15 +5,28 @@ import (
"github.com/xichen2020/eventdb/x/compare"
)

type nullResultGruop struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

Values calculation.ResultArray `json:"values"`
}

type nullResultGruopsJSON struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

@@ -0,0 +1,114 @@
package query
Copy link
Collaborator

Choose a reason for hiding this comment

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

file name typo

}

type timeBucketJSON struct {
StartAt int64 `json:"startAt"` // In nanoseconds
Copy link
Collaborator

Choose a reason for hiding this comment

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

startAtNanos?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sure.

@xichen2020 xichen2020 merged commit e5ab829 into master Feb 9, 2019
@xichen2020 xichen2020 deleted the xichen-marshal-json-results branch February 9, 2019 01:25
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

3 participants