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

Merge event data for campuses based on date #68

Merged
merged 8 commits into from
Apr 28, 2016

Conversation

kashav
Copy link
Member

@kashav kashav commented Apr 27, 2016

For start_time and end_time, would it be better to use seconds since midnight rather than ISO 8601 datetimes? I don't think there's any event that goes into the next day.

Also, thoughts on adding a duration key for each event?

#66

@qasim
Copy link
Member

qasim commented Apr 27, 2016

I'm in favour of moving start_time and end_time to seconds since midnight instead of ISO 8601. I've been pondering over the all of our date and time stuff and think that for one-day things like this, we should have a date string like YYYY-MM-DD and then time numbers for the times.

As for the API side of things, dates are proving to be a problem because as soon as we convert the date from UTC to local time zone, it's possible that the day changes since the original time is UTC 00:00:00, with a -4 or -5 offset it turns into the previous day.

I'm thinking of (on the cobalt-uoft/cobalt side of things) adding a date integer which would be an 8-digit wide YYYYMMDD. Then we could compare the integers when needing to compare dates, and compare integers again for when comparing times since those are already integers. This integer would only be for calculations and not shown in the output response. I'll write something up after my exams and see what people think.

@qasim
Copy link
Member

qasim commented Apr 28, 2016

This lets us just say that everyone should always assume that any date or time in Cobalt is considered to be ET, whether it is EST or EDT won't affect things. This way, we don't have to deal with Date object timezone issues, since it's only causing issues.

@qasim qasim merged commit 336b327 into cobalt-uoft:master Apr 28, 2016
@kashav kashav deleted the merge-dates branch April 28, 2016 03:53
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.

2 participants