-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add full UTF-8 support in RON incl. unicode identifiers #488
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #488 +/- ##
==========================================
+ Coverage 96.12% 98.14% +2.01%
==========================================
Files 77 78 +1
Lines 9890 10308 +418
==========================================
+ Hits 9507 10117 +610
+ Misses 383 191 -192
☔ View full report in Codecov by Sentry. |
@torkleyy I've finally gotten to rebasing #444. I would propose merging it after #438 lands - while I will try to find some performance improvements earlier, I think parsing in UTF8 cleans up the code so much that landing early and optimising in later commits could also be warranted. The last commit, 93f367c, adds an API-breaking change to better encode that ron only produces valid UTF-8 during serialising (by switching from |
@ModProg @torkleyy This PR is now (almost) ready to be merged. I'll review the changes again tomorrow and add a CHANGELOG entry. Apart from that, I think this is a very good new implementation (thank you so much @ModProg for doing most of the work towards it), which can be improved performance-wise in follow-up PRs if necessary |
@torkleyy We are also getting closer and closer to 100% code coverage ... maybe before releasing v0.9 that's the last thing after this PR that I take care of. |
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.
Some small cleanup is needed
Fixes #321
Superseded #444, which it rebases + adds 1aee993 to ensure that ron is always valid UTF-8
CHANGELOG.md