-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Use character array for utfstring #561
Use character array for utfstring #561
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #561 +/- ##
============================================
- Coverage 96.55% 96.31% -0.25%
- Complexity 2166 2192 +26
============================================
Files 87 87
Lines 5025 5072 +47
============================================
+ Hits 4852 4885 +33
- Misses 173 187 +14 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Maximilian Krög <maxi_kroeg@web.de>
8026eb5
to
51225c5
Compare
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.
That makes so much sense.
Maybe we should target 5.10.x ? |
If we are very strict about sementic versionning, keeping 6.0 makes sense because this PR removes accesses to public properties some projects could read directly, hence it causes a BC Break. But if we are loosely respecting it, having on 5.10 makes sense also as to me, this is very internal classes and it shouldn't be accessed by external projects, hence it should not cause any BC Break. I personnaly would go for 5.10.x, but I sometimes can be a cow-boy about it 😆 . |
Benchmarks indicate a nice performance gain, too.
Old:
New: