Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Set DateFormatHandling.IsoDateFormat for serializer. Fixes #351 #353

Merged
merged 1 commit into from
Feb 7, 2020
Merged

Conversation

olsh
Copy link
Contributor

@olsh olsh commented Feb 7, 2020

Related #351

According to the documentation, the default settings are used by JsonConvert method.

So, we have two options:

  • Override the default settings (this is done in the PR)
  • Create a new serializer with Create method

I believe that the second approach is preferable since we have more control over the serializing settings (I can update the PR).

@bruno-garcia Please let me know what do you think.

@bruno-garcia
Copy link
Member

I believe the Create method might be a better option because it's possible users set other global settings to their app which break Sentry.

Thanks a lot for helping with this!

@codecov-io
Copy link

codecov-io commented Feb 7, 2020

Codecov Report

Merging #353 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #353      +/-   ##
==========================================
+ Coverage   92.12%   92.13%   +<.01%     
==========================================
  Files          82       82              
  Lines        2109     2110       +1     
==========================================
+ Hits         1943     1944       +1     
  Misses        166      166
Impacted Files Coverage Δ
src/Sentry/Internal/JsonSerializer.cs 91.66% <100%> (+0.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b150d25...2415ce1. Read the comment docs.

@bruno-garcia
Copy link
Member

I'll merge this meanwhile ;)

@bruno-garcia bruno-garcia merged commit 7fe5221 into getsentry:master Feb 7, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants