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

[kohenkatz]

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]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (190948b) to head (484207c).

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #142   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          447       459   +12     
=========================================
+ Hits           447       459   +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cameracker]

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]

... 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]

@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]

@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.

[kohenkatz]

After what seems like forever, this PR now has tests!

Ready for review once again.

[dylan-bourque]

I'd hate to create a new major version for this, but this is pedantically a breaking change if any consumer has their own implementation of the Generator interface.

At a minimum, we should call out that there are 3 new methods on this interface so that consumers will have a pointer to the source of their broken builds.

[kohenkatz]

I think it is fine to remove from the interface, but that will mean we cannot use the package-level convenience methods, because those operate using DefaultGenerator which uses this interface.

This just means that to use the new methods users will need to create a custom generator.

[dylan-bourque]

I'm not against adding the methods to the interface, we just need to decide whether we bump to v6.x or we accept what is technically a breaking change in a v5.x minor release and "fix with words" for any consumers that have their own implementations.

[kohenkatz]

I think I'd go with jumping to 6.x. It's not such a big deal to make the change.

[dylan-bourque]

Easy enough for this code, but It's a much bigger deal for every consumer to have to update every import to s|v5|v6 😬

[kohenkatz]

Unfortunately, there's no way for us to know how many consumers are implementing the interface separately. Numbers like that would be very useful to this decision.

[dylan-bourque]

per pkg.go.dev, 749 public things import /v5. I know we have a bunch of internal modules at work that also import it.

I can confirm that we don't have any custom Generator implementations and someone could theoretically check all of those public importers, but it's impossible to know if any non-public consumers will break.

Personally, I feel like the odds are very low that this will actually be a breaking change and we should do a v5.x minor release with a note.

[kohenkatz]

Sounds good to me, but we probably should wait for at least one additional voice.

[dylan-bourque]

The code changes LGTM with the added tests, but we need to make a call on the breaking change to the Generator interface.

Super nit: I think I would prefer to only include the full details for a variant on just one of the New*() functions rather than have it repeated 4X across the 2 New*() functions and the 2 Gen methods.

[cameracker]

Thank you very much @kohenkatz . Regarding the discussion around releasing it as a minor or major, I think the likelihood of a consumer implementing a custom generator is unlikely due to the extremely niche functionality it offers, and I suspect we won't be impacting much by releasing it as a minor.