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

Added support for ISO8601 timestamps with time zones #146

Closed
wants to merge 3 commits into from

Conversation

sjauld
Copy link
Contributor

@sjauld sjauld commented Jun 19, 2018

constants.go Outdated
@@ -11,7 +11,8 @@ const (
annotationISO8601 = "iso8601"
annotationSeperator = ","

iso8601TimeFormat = "2006-01-02T15:04:05Z"
iso8601TimeFormat = "2006-01-02T15:04:05Z"
iso8601ExpandedTimeFormat = "2006-01-02T15:04:05-07:00"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious why not just use time.RFC3339?

Seems to work fine https://play.golang.org/p/1Rom5WsfCYO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm, I thought that I tried that and it didn't work. But yes that's a much better solution if it does work.

@aren55555
Copy link
Contributor

aren55555 commented Jul 25, 2018

I don't believe this works with 20180725T033557Z which is apparently also ISO 8601 compliant.

I'm more inclined to think this library should not take a stance on the actual wire format, but rather support any specification via custom types implementing the json.Marshaler and json.Unmarshaler interfaces.

@aren55555 aren55555 closed this Jul 25, 2018
@sjauld
Copy link
Contributor Author

sjauld commented Jul 26, 2018

So maybe the support for iso8601 should be removed altogether since it is incomplete and undocumented?

@aren55555
Copy link
Contributor

@sjauld IMO that's the most preferred path forward. I'd also suggest an additional README to call out how to custom types to support IS08601 or even RFC3339

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.

3 participants