Skip to content
This repository has been archived by the owner on Feb 7, 2023. It is now read-only.

Fixed TimeStamp resolution #4

Merged
merged 1 commit into from
Dec 25, 2018
Merged

Fixed TimeStamp resolution #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestNewClient(t *testing.T) {
func TestMessage_Client(t *testing.T) {
oldTimeStamp := TimeStamp
defer func() { TimeStamp = oldTimeStamp }()
TimeStamp = func() int { return 998877 }
TimeStamp = func() int64 { return 998877 }
c := New("foo-bar-baz")

t.Run("default", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions event.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ type Event struct {
APIKey string `json:"api_key"`
UserID string `json:"user_id"`
Intent string `json:"intent"`
TimeStamp int `json:"timestamp_millis,omitempty"`
TimeStamp int64 `json:"timestamp_millis,omitempty"`
Platform string `json:"platform,omitempty"`
Version string `json:"version,omitempty"`
Properties []EventProperty `json:"properties"`
}

// SetTimeStamp adds an optional "timestamp" value to the event
func (e *Event) SetTimeStamp(t int) *Event {
func (e *Event) SetTimeStamp(t int64) *Event {
e.TimeStamp = t
return e
}
Expand Down
4 changes: 2 additions & 2 deletions message.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ type Message struct {
APIKey string `json:"api_key"`
Type MessageType `json:"type"`
UserID string `json:"user_id"`
TimeStamp int `json:"time_stamp"`
TimeStamp int64 `json:"time_stamp"`
Platform string `json:"platform"`
Message string `json:"message,omitempty"`
Intent string `json:"intent,omitempty"`
Expand Down Expand Up @@ -90,7 +90,7 @@ func (m *Message) SetVersion(v string) *Message {
}

// SetTimeStamp overrides the message's "timestamp" value
func (m *Message) SetTimeStamp(t int) *Message {
func (m *Message) SetTimeStamp(t int64) *Message {
m.TimeStamp = t
return m
}
Expand Down
4 changes: 2 additions & 2 deletions timestamp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ import (
)

// TimeStamp returns the current time in UNIX milliseconds
var TimeStamp = func() int {
return int(time.Now().Unix())
var TimeStamp = func() int64 {
return time.Now().UnixNano()/1e6
}
4 changes: 2 additions & 2 deletions timestamp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import "testing"
func TestTimeStamp(t *testing.T) {
t.Run("default", func(t *testing.T) {
result := TimeStamp()
if result <= 0 {
t.Errorf("Expected non-zero timestamp, got %v", result)
if result < 10000000000 {
Copy link
Owner

Choose a reason for hiding this comment

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

Just to understand why you changed this: couldn't this still be <= 0 as all that it checks if the value is non-zero and the returned resolution does not really matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to make a basic check if resolution is in milliseconds. If >=1e10 is returned, than it's definitely milliseconds (1970 year), because if it's in seconds - it's 2286 year. I don't think anybody will use this library in 2286.
Yep, it's kind of a magic number. But there is no better way to check for resolution. Also, you can remove it, if you want, because I don't think anybody will break this simple function :).

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, I think that makes sense. Let's keep it like this.

t.Errorf("Expected non-zero timestamp in milliseconds, got %v", result)
}
})
}