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

Specify UTC time in Calendar widget #59

Merged
merged 1 commit into from
Apr 2, 2015
Merged

Specify UTC time in Calendar widget #59

merged 1 commit into from
Apr 2, 2015

Conversation

calumgunn
Copy link
Contributor

Nails #58.

@bensymonds
Copy link
Contributor

Ah ok I see, the _zonecode thing is saying it's UTC, so we just need to convert the time to that first. This makes sense to me, but I suppose let someone else check too.

@jgwhite
Copy link
Contributor

jgwhite commented Apr 2, 2015

I’m surprised that AddThisEvent wants the time in that format rather than ISO8601. Is that a quirk of their service?

@bensymonds
Copy link
Contributor

I think you have to create an account/log in to see any developer docs, so I couldn't be bothered to look. I agree it's a bit odd.

@jgwhite
Copy link
Contributor

jgwhite commented Apr 2, 2015

LGTM

@jgwhite
Copy link
Contributor

jgwhite commented Apr 2, 2015

Actually, @calumgunn one further suggestion — is there any sense in moving the utc conversion into the helper?

@calumgunn
Copy link
Contributor Author

@jgwhite Yes, there is.

@jgwhite
Copy link
Contributor

jgwhite commented Apr 2, 2015

Eres un hombre muy juicioso.

calumgunn added a commit that referenced this pull request Apr 2, 2015
Specify UTC time in Calendar widget
@calumgunn calumgunn merged commit 1b42798 into master Apr 2, 2015
@calumgunn calumgunn deleted the utc_calendar branch April 2, 2015 15:54
@jgwhite
Copy link
Contributor

jgwhite commented Apr 2, 2015

💥

@jgwhite jgwhite deployed to pair-withassociates April 2, 2015 15:55 Active
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