-
Notifications
You must be signed in to change notification settings - Fork 81
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
Json appender #458
Json appender #458
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks phenomenal! Thanks for putting up with the revisions.
After conflicts are fixed, this is good to merge!
There is some separate work we could do later to optimize performance.
@davidlar This is good to merge when you get a chance to fix conflicts! |
Great! I thought about reusing the string builder more instead of creating new ones. Don't know if it will affect performance or not. |
@davidlar Thanks for your persistence! We finally got this merged in. One of the main things, I think, would be using low-level 'while' loops and working directly with |
This is a complete rework of #424 after review by @jdegoes.