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 DateTime serialized as FileTime #1731

Closed
wants to merge 10 commits into from

Conversation

itaibh
Copy link
Contributor

@itaibh itaibh commented Jan 1, 2018

No description provided.

@shiftkey
Copy link
Member

shiftkey commented Jan 1, 2018

@itaibh can you provide some more context for this change? Also, this might be worth upstreaming to SimpleJson if it's useful more broadly...

@itaibh
Copy link
Contributor Author

itaibh commented Jan 1, 2018

In some cases GitHub sends wrong date format for the "created_at" and "pushed_at" fields. This is a GitHub bug, probably, but still, such a convention should be supported (and it would have saved me hours of debugging).

Back in 2015 there was an open issue on this, which I commented on today, but I can't find it right now. I'm sending this from my phone, and don't have the full attention to provide more info right now.

@shiftkey
Copy link
Member

shiftkey commented Jan 1, 2018

@itaibh thanks for the extra info. I found #825 where you commented earlier.

This is a GitHub bug, probably, but still, such a convention should be supported (and it would have saved me hours of debugging).

In your other comment you mention this:

Also, from the timing it seems the problem had returned only recently (about two weeks ago).

If this is a recent regression then I'd love to try and chase this down, because we should be consistent with these fields. If you can point me to a repository which fired a webhook with the wrong format for created_at and pushed_at in the payload I can chase that up internally and confirm it's been resolved while we discuss this fix.

@itaibh
Copy link
Contributor Author

itaibh commented Jan 2, 2018 via email

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