Skip to content

Conversation

@xavdid-stripe
Copy link
Member

Fixes #1101 by using the native UUID implementation, if available. Added a (very) basic test as a starter.

@richardm-stripe
Copy link
Contributor

LGTM, I pushed a commit and wrote one of the tests you described

@xavdid-stripe
Copy link
Member Author

@richardm-stripe Ok, I pushed a tweak to your test so we only mock when the function was otherwise available.

Mind giving it a quick review when you have a chance?

@xavdid-stripe xavdid-stripe enabled auto-merge (squash) October 18, 2022 17:15
@xavdid-stripe xavdid-stripe merged commit c9d9522 into master Oct 18, 2022
@xavdid-stripe
Copy link
Member Author

Ope! Turns out auto-merge doesn't wait for reviewers 🤔 Probably still take a peek and lmk if you see anything we should tweak. Thank you!

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.

Math.random() is not a cryptographically secure way to generate UUIDv4s

3 participants