-
Notifications
You must be signed in to change notification settings - Fork 436
impl(spanner): do Uuid to/from string conversions using absl::uint128 #15057
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
Conversation
Instead of processing the high and low 64 bits of the Uuid separately, just use the 128-bit value directly. Also simplify the Uuid tests. Fixes googleapis#15043.
|
/gcbrun |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15057 +/- ##
=======================================
Coverage 92.90% 92.91%
=======================================
Files 2354 2354
Lines 210374 210365 -9
=======================================
+ Hits 195443 195452 +9
+ Misses 14931 14913 -18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
It looks like the cmake-oldest-deps build is old enough to not support the deduced-type `%v` specifier.
|
/gcbrun |
|
The tests don't look like they're operating as planned: Please assist. |
|
/gcbrun |
|
I don't know how/when you folks triage PRs these days, but it would be nice to get some attention. |
scotthart
left a comment
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.
Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @devbww)
Sorry about the delay, spinning lots of plates these days. |
Instead of processing the high and low 64 bits of the Uuid separately, just use the 128-bit value directly.
Also simplify the Uuid tests.
Fixes #15043.
This change is