-
Notifications
You must be signed in to change notification settings - Fork 300
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 LazyWorkerInterface #3038
Add LazyWorkerInterface #3038
Conversation
5bdfd45
to
7e7b195
Compare
This is a new WorkerInterface class that will be used in the internal PR, see comments to understand what it does.
7e7b195
to
2c56853
Compare
template <typename Func> | ||
class LazyWorkerInterface final: public WorkerInterface { | ||
public: | ||
LazyWorkerInterface(Func func): func(kj::mv(func)) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking nit: might consider marking this explicitly KJ_DISALLOW_COPY_AND_MOVE
but not critical with having the KJ_ASSERT_NONNULLs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea copies would be bad but they are implicitly disabled by virtue of having an Own member.
I don't see why moves should be a problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Largely just an explicit hygiene thing for me... kind of a safeguard against someone changing any of the methods later and forgetting a KJ_ASSERT_NONNULL
. But I'm being a bit extra paranoid :-)
This is a new WorkerInterface class that will be used in the internal PR, see comments to understand what it does.