gofrs / uuid

A UUID package for Go
MIT License
1.55k stars 108 forks source link

Add "AtTime" generators for V1, V6, and V7 #142

Open kohenkatz opened 2 months ago

kohenkatz commented 2 months ago

This PR implements the request in #84 to be able to create UUIDs at specific known timestamps, in addition to the current time.

Although that request only mentions UUIDv1, this PR also includes the other two time-based UUID versions, namely v6 and v7.

To prevent code duplication, this PR reimplements NewV1, NewV6, and NewV7 to each call their respective NewV<X>AtTime with the provided EpochFunc, which defaults to providing the current time.

This PR will remain a draft until I have time to write some tests...

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (2f6f9f4) to head (ee708e7).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #142 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 5 5 Lines 447 453 +6 ========================================= + Hits 447 453 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

cameracker commented 2 months ago

Nice. Looks good so far. I think it was the right call to add some additional functions. As a mega nit, what do you think of the convention NewVNFromTime()?

kohenkatz commented 2 months ago

... what do you think of the convention NewVNFromTime()?

Personally, I like At over From for three reasons:

  1. To me, "From" implies some kind of derivation/modification, while "At" implies that the time is used exactly as-is.
  2. I am used to other frameworks that use At as an indicator that something is a timestamp. For example, Laravel's created_at, updated_at, and deleted_at database fields.
  3. It's slightly shorter to type.

All that said, I really don't care if you want to change it.

cameracker commented 2 months ago

@kohenkatz Dependabot updated the go-setup github action, and I noted we were testing against older versions of go so I took the opportunity to kick them up to latest and latest -1. Might be worth rebasing against master :)

cameracker commented 1 week ago

@kohenkatz Hi, heads up, I fat fingered the update branch button. You'll want to do a pull when you pick this back up again. Apologies.