Skip to content

Conversation

@JamieMagee
Copy link
Member

@tobysmith568
Copy link
Contributor

Ah looks good @JamieMagee. Is it worth adding a test to each converter with a datetime in this format 2022-01-13T09:21:34.000+00:00 as it's also a format that the GitHub API sometimes returns? I also wonder if the unix timestamps in their tests should be ints rather than strings?

@JamieMagee
Copy link
Member Author

I added tests for the other format. Unfortunately, JsonSerializer.Deserialize only takes string as a parameter so I can't pass an int directly, but I am passing the JSON equivalent of an int @"123" instead of @"""123"""

@tobysmith568
Copy link
Contributor

Ah yeah that makes sense :)

@JamieMagee JamieMagee merged commit 1a6ce29 into main Feb 2, 2022
@JamieMagee JamieMagee deleted the converter-tests branch February 2, 2022 17:42
@JamieMagee
Copy link
Member Author

Super! I've merged this and cut a release with your changes 🎉

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