Skip to content
This repository has been archived by the owner on May 18, 2023. It is now read-only.

Add WithDeadline/WithTimeout context functions #41

Merged
merged 5 commits into from Nov 15, 2021

Conversation

morigs
Copy link
Contributor

@morigs morigs commented Nov 12, 2021

This is renewal of #20
It's rebased on latest changes from master now.
I've also added usage of Until instead of Sub as well as some basic tests

Copy link
Collaborator

@djmitche djmitche left a comment

Choose a reason for hiding this comment

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

This looks good! Thanks!

What do you think about also adding clock.WithCancel? Using this will require calling clock.WithFoo instead of context.WithFoo, and it might be easier for users to remember to change all uses of context.WithFoo rather than just two of the three. WDYT?

@morigs
Copy link
Contributor Author

morigs commented Nov 15, 2021

Tbh I think it's better to provide a facade for the whole package. But historically, clock hide only some types and functions from time. Probably the same logic should apply for context.
But on the other hand, your point on usage consistency is also valid. So I don't have a strong opinion.
The only thing I can say - adding WithCancel will require some rework because initial implementation of mocked methods were simplified to avoid some complexity of the WithCancel.

@djmitche
Copy link
Collaborator

OK, if it's simpler this way, let's just do it this way :)

@djmitche djmitche merged commit 68df829 into benbjohnson:master Nov 15, 2021
@morigs
Copy link
Contributor Author

morigs commented Nov 16, 2021

Ok, let me know if there will be need for WithCancel.
Looking forward for the next release!

@djmitche
Copy link
Collaborator

We'll see if an issue gets filed. I'll make a new release soon (on the scale of days)!

@djmitche
Copy link
Collaborator

https://pkg.go.dev/github.com/benbjohnson/clock@v1.3.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants