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

ExceptionLayoutRenderer - Support Serialize Format #2357

Merged

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Oct 14, 2017

Made some minor optimizations:

  • Skip preparation for special handling of AggregateException if MaxInnerExceptionLevel = 0 (Default)
  • Skip allocation of extra StringBuilder-objects for each render-format, but just use the provided one.

@snakefoot snakefoot force-pushed the ExceptionLayoutRendererSerialize branch from ee81318 to 8b8de12 Compare October 14, 2017 16:44
@snakefoot
Copy link
Contributor Author

snakefoot commented Oct 14, 2017

Suddenly became a very good test of the Json-serializer. Discovered that NetStandard attempted to access NonPublic properties because of GetRuntimeProperties() includes everything.

Also curious if the internal Json-Converter should not recognize Fields and not just Properties?

@snakefoot snakefoot force-pushed the ExceptionLayoutRendererSerialize branch from 97dde15 to 88ef1b7 Compare October 14, 2017 18:07
@304NotModified
Copy link
Member

Also curious if the internal Json-Converter should not recognize Fields and not just Properties?

Nog sure. AFAIK anonymous objects creates properties and so can't think of a cade where a field is needed. Bit maybe it should be an option.

@snakefoot
Copy link
Contributor Author

maybe it should be an option.

Will leave this task for future generations :)

@304NotModified
Copy link
Member

👍

@snakefoot snakefoot force-pushed the ExceptionLayoutRendererSerialize branch from 88ef1b7 to 126bd50 Compare October 14, 2017 18:34
@codecov
Copy link

codecov bot commented Oct 14, 2017

Codecov Report

Merging #2357 into master will decrease coverage by <1%.
The diff coverage is 96%.

@@           Coverage Diff           @@
##           master   #2357    +/-   ##
=======================================
- Coverage      82%     82%   -<1%     
=======================================
  Files         318     318            
  Lines       22790   22788     -2     
  Branches     2782    2783     +1     
=======================================
- Hits        18684   18681     -3     
- Misses       3403    3406     +3     
+ Partials      703     701     -2

@snakefoot snakefoot force-pushed the ExceptionLayoutRendererSerialize branch 6 times, most recently from ee83b81 to 86a5777 Compare October 14, 2017 20:58
@snakefoot snakefoot force-pushed the ExceptionLayoutRendererSerialize branch from 86a5777 to e0627a8 Compare October 14, 2017 21:03
@304NotModified 304NotModified merged commit 3bcd707 into NLog:master Oct 14, 2017
@304NotModified 304NotModified added this to the 4.5 beta 7 milestone Oct 14, 2017
@snakefoot snakefoot mentioned this pull request Nov 23, 2017
@304NotModified 304NotModified added the documentation done all docs done (wiki, api docs, lists on nlog-project.org, xmldocs) label Jan 14, 2018
@snakefoot snakefoot deleted the ExceptionLayoutRendererSerialize branch April 4, 2020 17:42
@snakefoot snakefoot modified the milestones: 4.5 beta 7, 4.5 Aug 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation done all docs done (wiki, api docs, lists on nlog-project.org, xmldocs) feature needs documentation on wiki
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants