Skip to content

Allow other formats when unmarshalling time#1964

Open
ItalyPaleAle wants to merge 1 commit into
99designs:masterfrom
ItalyPaleAle:unmarshal-time-format
Open

Allow other formats when unmarshalling time#1964
ItalyPaleAle wants to merge 1 commit into
99designs:masterfrom
ItalyPaleAle:unmarshal-time-format

Conversation

@ItalyPaleAle

Copy link
Copy Markdown

This PR makes the method that unmarshalls times a bit more flexible.

Currently, only dates that are in full RFC339 format are supported, strictly. This adds a couple more formats relaxing the parser:

  • 2006-01-02 15:04:05.999999999 -> allows things like 2022-02-10 10:20:30 (nanoseconds are optional)
  • 2006-01-02 -> allowing things like 2022-02-10 (which defaults to midnight)

When a time zone is not specified, it defaults to UTC (as the default for time.Parse)

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

@giautm

giautm commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

I think you should mapping Time to your custom scalar instead.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.04%) to 74.46% when pulling 9a0650b on ItalyPaleAle:unmarshal-time-format into d7da5b0 on 99designs:master.

@ItalyPaleAle

Copy link
Copy Markdown
Author

I think you should mapping Time to your custom scalar instead.

Certainly an option. I still thought of submitting this as it doesn't necessarily change the behavior of the un-marshaller but just adds some more flexibility, for example adding support for formats that work on Postgres and many other places.

Up to the maintainers if they want to consider this or not :)

@giautm

giautm commented Feb 14, 2022

Copy link
Copy Markdown
Contributor

This PR changes the format to accept date-only layout (2006-01-02) can break other applications that don't expect it.

PS: Time layout format was a very-old issue in the Golang repository. :(

@StevenACoffman StevenACoffman self-requested a review as a code owner March 17, 2026 11:19
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.

6 participants