Conversation
| * type binding to an {@link UntypedNexusClientHandle} (returned by {@link | ||
| * NexusClient#getHandle(String)}) by calling one of the {@link #fromUntyped} factories. | ||
| */ | ||
| public interface NexusClientHandle<R> extends UntypedNexusClientHandle { |
There was a problem hiding this comment.
| public interface NexusClientHandle<R> extends UntypedNexusClientHandle { | |
| public interface NexusOperationHandle<R> extends UntypedNexusOperationHandle { |
Please keep consistent naming with other Handles like https://github.com/temporalio/sdk-java/blob/master/temporal-sdk/src/main/java/io/temporal/client/ActivityHandle.java
There was a problem hiding this comment.
The nexus doc had "NexusOperationHandle". I didn't use that name as there was already a NexusOperationHandle class and I didn't want to duplicate it. Even if in a different package, that just seemed confusing.
The reason I went to NexusClient in a lot of class names was to avoid naming collisions like that - again, even if in different packages it seemed confusing, and I wanted some form of consistency. So I named this NexusClientHandle to show that it was linked to the other NexusClient classes.
That being said, I do want to be consistent, but might have to check in with you and talk this one out. Maybe we can make these NexusOperationExecutionHandle and UntypedNexusOperationExecutionHandle, but then we lose the link to the other NexusClient classes - though would we use these for all Nexus operations so maybe we don't need such a link?
There was a problem hiding this comment.
Renamed to NexusOperationHandle as per conversation - we feel that the names being in different packages that have different use cases which should never be mixed should be sufficient to avoid confusion, especially as the user will get this returned to them and won't be creating these classes.
Leaving this conversation unresolved to make sure the SDK team sees it and can weigh in!
|
Reviewed most of the public API, didn't get to into the tests or implementation for now since some stuff will likely change. |
…nt, typed and untyped service clients)
What was changed
Why?
Checklist
Closes
How was this tested: