Implement SDFC date <-> Go time type #62
Conversation
| CreatedById string `force:",omitempty" json:",omitempty"` | ||
| LastModifiedDate string `force:",omitempty" json:",omitempty"` | ||
| LastModifiedDate *Time `force:",omitempty" json:",omitempty"` |
There was a problem hiding this comment.
These have to be pointers rather than embedded structs for omitempty to be respected.
| ) | ||
|
|
||
| const SFTIMEFORMAT1 = "2006-01-02T15:04:05.000-0700" | ||
| const SFTIMEFORMAT2 = "2006-01-02T15:04:05.999Z" |
There was a problem hiding this comment.
| const SFTIMEFORMAT2 = "2006-01-02T15:04:05.999Z" | ||
|
|
||
| // Represents a SFDC Date. Implements marshaling / unmarshaling as a Go Time. | ||
| type Time time.Time |
There was a problem hiding this comment.
There are pros and cons to this formulation vs.
type struct Time {
time.Time
}Feedback welcome.
| // Convenience Go time.Time converstion. | ||
| func (t Time) Time() time.Time { | ||
| return time.Time(t) | ||
| } |
There was a problem hiding this comment.
This should possibly be:
func (t* Time) Time() time.Time {
if t == nil {
return time.Time{}
}
return time.Time(*t)
}
|
@nimajalali any interest in this? |
|
Is this project active? |
There was a problem hiding this comment.
@dragonsinth Thanks for the PR. I haven't been monitoring this repo closely but will go through it now.
I was hesitant to merge PRs before due to backward compatibility. Now that Go has go mod for dependency management I feel better about merging PRs.
| return nil | ||
| } | ||
|
|
||
| var _ forcejson.Unmarshaler = (*Time)(nil) |
There was a problem hiding this comment.
Are these required? What are they accomplishing?
There was a problem hiding this comment.
They're just compile time interface checks. We're verifying that the type implements custom json marshal/unmarshal.
sobjects/date_test.go
Outdated
| package sobjects | ||
|
|
||
| import ( | ||
| "testing" |
There was a problem hiding this comment.
Minor but can we run goimports on this?
|
updated and rebased |
|
@dragonsinth I fixed the test account and moved CI to github actions. Do you mind rebasing with master then we can merge? |
|
Re-rebased! |
|
Merged! Thx |
|
Awesome thanks! |
See #38