-
Notifications
You must be signed in to change notification settings - Fork 284
Set Guid version to 8 and embed reserve 4-bits for our own version "1" for now. #5974
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rel/4.0 #5974 +/- ##
==========================================
Coverage ? 65.03%
==========================================
Files ? 579
Lines ? 32355
Branches ? 0
==========================================
Hits ? 21041
Misses ? 11314
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
From discussion with @tannergooding, the version field can be used by tools to interpret Guid in certain ways. https://www.rfc-editor.org/rfc/rfc9562.html#name-uuid-version-1 Version 1 for example is for UTC timestamps and network card MAC address encoding. If we want to embed our own version while following the RFC properly, we should use version 8, and reserve other bits for embedding our "own" version. Note that in the past, I think we never followed the RFC accurately, as we just hashed and passed that to In addition, XxHash128 output is big endian, so it's best if we do Note that there is not much of a concern of us using extra bits for reserving our version with regards to collision probability, as it's still very unlikely to practically see collisions. I think that summarizes our discussion @tannergooding (hopefully accurately/correctly). |
src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs
Outdated
Show resolved
Hide resolved
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.
Approving but I'll let you merge once you get the confirmation.
src/Adapter/MSTestAdapter.PlatformServices/ObjectModel/UnitTestElement.cs
Outdated
Show resolved
Hide resolved
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.
Will rewrite in a way that touches single bytes.
|
Fixes #5957