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

add UUID version 7 generator #191

Closed
wants to merge 1 commit into from

Conversation

tsurowiec
Copy link
Contributor

Description

added a UUID Version 7 Generator

Motivation and context

with the introduction of new UUID Types, there is a better way then Percona's solution to achieve the sortable, time-based UUIDs that can be efficiently used as primary keys.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • I have added tests to cover my changes.

@codecov
Copy link

codecov bot commented Nov 6, 2022

Codecov Report

Merging #191 (2bd21fe) into main (6867db3) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 2bd21fe differs from pull request most recent head 9498c0d. Consider uploading reports for the commit 9498c0d to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##                main      #191   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity        60        61    +1     
===========================================
  Files              5         6    +1     
  Lines            105       107    +2     
===========================================
+ Hits             105       107    +2     
Impacted Files Coverage Δ
src/UuidV7Generator.php 100.00% <100.00%> (ø)

@tsurowiec
Copy link
Contributor Author

@ramsey tests fail on PHP below 8.0. Uuid::uuid7() is not available there. I don't know how to proceed. Any hints?

@ramsey
Copy link
Owner

ramsey commented Nov 6, 2022

Good question. I see two options:

  1. Drop support for PHP < 8.0
  2. Throw an exception in in the constructor for UuidV7Generator if !method_exists(Uuid::class, 'uuid7')

The second option seems like it might be the best to avoid dropping support for < PHP 8.0 at the moment.

@tsurowiec
Copy link
Contributor Author

@ramsey updated PR, I hope skipped tests are allowed in workflow. I had to go for full class name instead of ::class due to 5.4 support.

ramsey pushed a commit that referenced this pull request Dec 20, 2022
@ramsey
Copy link
Owner

ramsey commented Dec 20, 2022

Thank you for contributing! 🎉

I committed this in 4b56c72

@ramsey ramsey closed this Dec 20, 2022
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