-
Notifications
You must be signed in to change notification settings - Fork 242
Rename NpgsqlUuid7ValueGenerator to NpgsqlSequentialGuidValueGenerator and make it public #3255
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
…r and make it public Follow-up to npgsql#3249
And I just moved it from public to internal from your suggestion
Anyway if you want to allow for that scenario should be fine |
Yeah, I indeed didn't have the non-key scenario in mind...! |
@ChrisJollyAU, @roji, should we now provide appropriate summaries on the public members, instead of the pubternal summaries? @roji I do like the naming consistency with EF's counterpart, but I dislike the "sequential" claim so long as the implementation generates nonsequential IDs intra-millisecond. The name adds to the astonishment when it comes to that property. For your consideration. |
Sure, feel free to submit a PR if you want to add docs (though I think there's little value in it). I really think it's fine to use the word sequential here; that's not the same as saying "strictly monotonic" - the values produced really are sequential (in the sense that they're not random). |
Technically, I disagree with the statement that the values really are sequential: if you produce a handful at once, their order literally will be random, not sequential. But I mainly wanted to check that you're aware and satisfied. |
…r and make it public (npgsql#3255) Follow-up to npgsql#3249
EF only automatically configured client-side GUID generation for key GUID properties; other non-key GUID properties do not get configured by default. Since users may want to explicitly configure that behavior (see explicit call-out on this in the docs), the new UUID7 generator should be public and more properly named.
@ChrisJollyAU @Timovzl does this (and the new naming) make sense to you?
Follow-up to #3249