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

Context WithDeadline/WithTimeout methods #20

Closed
wants to merge 2 commits into from

Conversation

aviddiviner
Copy link
Contributor

Hi. I don't know what you think of this, but I make heavy use of contexts in my code and I needed to test some timeouts using the mock clock. So I added on two methods to the Clock interface, as well as their mock implementations (inspired from the core library context.WithDeadline code).

Let me know what you think, I'm happy to tweak things. Of course, we can also add tests, etc.

}
ctx := &timerCtx{clock: m, parent: parent, deadline: deadline, done: make(chan struct{})}
propagateCancel(parent, ctx)
dur := deadline.Sub(m.Now())
Copy link
Contributor Author

@aviddiviner aviddiviner Feb 1, 2020

Choose a reason for hiding this comment

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

The core library context code uses time.Until(deadline) but this is the same thing. Might be worth also adding a Until method to the Clock interface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That was done in #38.

@djmitche
Copy link
Collaborator

djmitche commented Nov 1, 2021

@aviddiviner would you mind rebasing this and I can take a closer look?

@morigs
Copy link
Contributor

morigs commented Nov 11, 2021

Do you need any help? I also really need this feature

@djmitche
Copy link
Collaborator

If you'd like to make a new PR with this rebased, that'd be great. And, have a look at the implementation and make any adjustments you'd like -- I haven't really looked yet, to be honest, so there may be issues with the approach.

@morigs
Copy link
Contributor

morigs commented Nov 12, 2021

What do you think on moving these functions into subpackage? Let's say github.com/benbjohnson/clock/context
I can't help feeling that this is not the right place for them. Clock intended for mocking time package and these functions are related to the context package.

@djmitche
Copy link
Collaborator

Closed in favor of #41.

@djmitche djmitche closed this Nov 12, 2021
@aviddiviner
Copy link
Contributor Author

Thanks all. Sorry I was a bit late to respond, but I see you handled it 😊

@djmitche
Copy link
Collaborator

Yay open source! Thanks for the contribution :D

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.

3 participants