Deprecate Worker() and Client() in favor of NewWorker() DefaultClient() #95
Conversation
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
temporaltest/server.go
Outdated
| if ts.defaultClient == nil { | ||
| ts.defaultClient = ts.NewClientWithOptions(ts.defaultClientOptions) | ||
| } | ||
| return ts.defaultClient |
There was a problem hiding this comment.
The reason this function wasn't initially named NewClient was because it actually memoizes the client being returned. But I think it's ok to "lie" here and simply reuse the same client for each call... anyone who really wants a newly instantiated client will still be able to use NewClientWithOptions. This leaves the door open for us to create a separate client for each invocation if we need to for whatever reason.
There was a problem hiding this comment.
What do you think about naming this DefaultClient? This seems more in line with what the function is actually doing. If users want a custom, newly instantiated client they can use NewClientWithOptions like you propose.
|
|
Signed-off-by: grantfuhr <grant.fuhr@datadoghq.com>
grantfuhr
changed the title
Co-authored-by: Jacob LeGrone <jlegrone@users.noreply.github.com>
Signed-off-by: grantfuhr grant.fuhr@datadoghq.com
What changed?
This PR deprecates Worker() and Client() functions in the TemporalTest package.
Why?
The naming convention NewWorker() and NewClient() is more consistent with the rest of the project.
How did you test it?
Yes. Tested locally
Potential risks
Some Temporaltest tests may fail.
Is hotfix candidate?
No