Skip to content
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

[redbean] Add UuidV4 method #1140

Merged
merged 18 commits into from
Apr 12, 2024
Merged

[redbean] Add UuidV4 method #1140

merged 18 commits into from
Apr 12, 2024

Conversation

solisoft
Copy link
Contributor

@solisoft solisoft commented Apr 9, 2024

This PR add the UuidV4 Lua method.

@mrdomino mrdomino self-requested a review April 10, 2024 04:54
Copy link
Collaborator

@mrdomino mrdomino left a comment

Choose a reason for hiding this comment

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

Much of this is as discussed in Discord, as well as a couple minor nits.

Looks good overall.

I'm not totally opposed to checking in the unrolled loop, I just want to push back once more, particularly in light of needing to use random() (which might change the performance margins already.)

tool/net/lfuncs.c Outdated Show resolved Hide resolved
tool/net/lfuncs.c Show resolved Hide resolved
tool/net/help.txt Outdated Show resolved Hide resolved
test/tool/net/uuidv4_test.lua Outdated Show resolved Hide resolved
test/tool/net/uuidv4_test.lua Outdated Show resolved Hide resolved
@mrdomino

This comment was marked as outdated.

Copy link
Collaborator

@mrdomino mrdomino left a comment

Choose a reason for hiding this comment

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

Looks good, just update the copyright year and address the static comment and I'm happy with this.

tool/net/lfuncs.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@mrdomino mrdomino left a comment

Choose a reason for hiding this comment

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

LGTM

@mrdomino mrdomino merged commit 39dde41 into jart:master Apr 12, 2024
5 checks passed
jart pushed a commit that referenced this pull request Jul 1, 2024
To Complete #1140 add UUID version 7 to Redbean
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants