Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add a TimeWithZone newtype, similar to the DotNetTime newtype, wrapping the default time format generated by Rails. #58

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

mike-burns commented Dec 29, 2011

I've needed to handle the time format produced by Rails by default a couple of times now (Github and Trajectory APIs), so I'm submitting it for more general use.

Add a TimeWithZone newtype, similar to the DotNetTime newtype, wrappi…
…ng the default time format generated by Rails.
Collaborator

hvr commented Dec 29, 2011

weird... rails really uses 3-letter timezones as in "2011-12-29T08:34:25UTC"?

Just wondering, why didn't they just use the easier to parse ES5 subset of ISO8601 which uses resolved numeric offsets?

Contributor

mike-burns commented Dec 29, 2011

While investigating your comment I discovered that they do use numeric offsets ... sometimes.

It starts with TimeWithZone#as_json: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/time_with_zone.rb#L133

But the real fun happens in TimeWithZone#formatted_offset, which special-cases UTC: https://github.com/rails/rails/blob/master/activesupport/lib/active_support/time_with_zone.rb#L97-L99

Will re-work this patch today in light of this.

Contributor

mike-burns commented Jan 4, 2012

Revamped, with a property test. Thanks for making me look into that, @hvr.

Collaborator

hvr commented Jan 8, 2012

Btw, to me it looks as if your ToJSON instance for your newtyped ZonedTime produces almost(!) the format as specified in the ECMA-262 specification (see also issue #1), so it might be useful to provide proper ToJSON/FromJSON instances for the "naked" ZonedTime types which follow the strict ECMA-262 spec, while providing newtypes for the Rails-format handling the cases when the format is not compatible w/ ECMA-262.

Owner

bos commented Jan 21, 2012

Sounds like a good suggestion. I'll wait on another refresh before pulling?

Contributor

mike-burns commented Jan 22, 2012

Sounds good, I'll try to get to this tomorrow at HacBoston.

Contributor

mike-burns commented Jan 22, 2012

Submitted #64, which actually takes care of this in a much better way.

@mike-burns mike-burns closed this Jan 22, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment