Skip to content

Proposal to generate UUIDs using RandomNumberGenerators #1271

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Aug 6, 2025
Merged

Proposal to generate UUIDs using RandomNumberGenerators #1271

merged 7 commits into from
Aug 6, 2025

Conversation

FranzBusch
Copy link
Member

This PR adds a proposal to generate UUID's using RandomNumberGenerators

@FranzBusch FranzBusch force-pushed the fb-random-uuid branch 2 times, most recently from fb5c82f to df12e88 Compare April 24, 2025 18:59
itingliu
itingliu reviewed Apr 25, 2025
View reviewed changes
Copy link
Contributor

@itingliu itingliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need two proposal files

@FranzBusch FranzBusch force-pushed the fb-random-uuid branch 2 times, most recently from 8f18f07 to d9ffe2f Compare April 29, 2025 19:46
stephentyrone
stephentyrone reviewed Apr 29, 2025
View reviewed changes
@itingliu
Copy link
Contributor

@swift-ci please test

@itingliu itingliu requested a review from jmschonfeld July 28, 2025 21:42
@FranzBusch
Copy link
Member Author

@swift-ci please test

@FranzBusch
Copy link
Member Author

@itingliu I rebased the PR against latest main and migrated the tests to swift-testing

@itingliu
Copy link
Contributor

@swift-ci please test

@itingliu
Copy link
Contributor

I am very sorry that I let this implementation slip and did not get to this in time for 6.2. We'll have to defer it to 6.3 since we won't be able to merge it at this time.

@FranzBusch
Copy link
Member Author

I am very sorry that I let this implementation slip and did not get to this in time for 6.2. We'll have to defer it to 6.3 since we won't be able to merge it at this time.

No worries. The PR is targeting main and I don't have a need to get it cherry picked.

jmschonfeld
jmschonfeld approved these changes Jul 29, 2025
View reviewed changes
Copy link
Contributor

@jmschonfeld jmschonfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation LGTM aside from the availability update!

itingliu
itingliu approved these changes Jul 29, 2025
View reviewed changes
@FranzBusch
Copy link
Member Author

@jmschonfeld It looks like 6.3 is not yet defined do you want me to do this or are we landing this in a separate PR?

@jmschonfeld
Copy link
Contributor

@jmschonfeld It looks like 6.3 is not yet defined do you want me to do this or are we landing this in a separate PR?

That's almost landed in #1454 - just waiting on another check or two to finish and then will merge that. Sorry for the delay on that!

@FranzBusch
Copy link
Member Author

@jmschonfeld I pulled in just the Package.swift changes here as well. Feel free to merge this PR once yours landed or give me a ping.

@jmschonfeld
Copy link
Contributor

@swift-ci please test

weissi
weissi reviewed Aug 3, 2025
View reviewed changes
weissi
weissi reviewed Aug 3, 2025
View reviewed changes
weissi
weissi reviewed Aug 3, 2025
View reviewed changes
weissi
weissi reviewed Aug 3, 2025
View reviewed changes
@FranzBusch FranzBusch requested a review from weissi August 4, 2025 13:59
weissi
weissi approved these changes Aug 4, 2025
View reviewed changes
Copy link
Contributor

@weissi weissi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thnks looks great and much more readable now IMHO

@FranzBusch
Copy link
Member Author

@swift-ci please test

@FranzBusch FranzBusch merged commit 8ce9756 into main Aug 6, 2025
18 checks passed
@FranzBusch FranzBusch deleted the fb-random-uuid branch August 6, 2025 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stephentyrone stephentyrone stephentyrone left review comments

@weissi weissi weissi approved these changes

@jmschonfeld jmschonfeld jmschonfeld approved these changes

@itingliu itingliu itingliu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@FranzBusch @itingliu @jmschonfeld @weissi @stephentyrone