Use DateTimeOffset Instead of Int64/long for Dates #119
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using DateTimeOffset instead of DateTime will allow us to force use of UTC with the API even if the consumer wants to use local timezones.
Contributor
Author
|
Just as an FYI, I've run into a couple of bugs with these changes that I'm working through now. I'll push any changes here and advise when everything is working properly. |
No need to forward to the next token. We can just get the value of the current property.
kmossco
suggested changes
Jun 15, 2018
Contributor
kmossco
left a comment
kmossco
left a comment
There was a problem hiding this comment.
Hey @DotJoshJohnson! So sorry for the delay in getting back to you here. Can you please add also how this changes the instructions we have in the readme? And can you squash the commits into just one? Thank you in advance! 🤜 🤛
Contributor
Author
|
No worries! I've actually gotten sidetracked with other priorities myself. I'll jump back in here as soon as I can to get these changes in place. Thanks for the consideration! |
Contributor
|
@DotJoshJohnson no rush at all. Thank you once again. 🙇 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've found myself adding helper classes to convert the UNIX timestamps in the Intercom models to
DateTimeobjects for use with our business logic. This PR changes allInt64/longimplementations in the models toDateTimeOffset(orDateTimeOffset?where applicable) and implements a JsonConverter to ensure dates are always represented as UNIX timestamps in UTC when serialized to JSON for the API.If this PR doesn't align with the direction of the project, feel free to simply close it. I just wanted to share in case anyone found it useful. Thanks!