pkuwm commented on a change in pull request #1344:
URL: https://github.com/apache/helix/pull/1344#discussion_r482739928
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -533,7 +533,7 @@ public void invoke(NotificationContext changeContext)
throws Exception {
}
}
- private void subscribeChildChange(String path, NotificationContext.Type
callbackType) {
+ private List<String> subscribeChildChange(String path,
NotificationContext.Type callbackType) {
if (callbackType == NotificationContext.Type.INIT
Review comment:
I thought about it. There is still a possibility that
`childrenSubscribeResult.getChildren()` returns `null`. The caller still needs
to do the `null` check. So I just make it consistent with
`zkClient.subscribeChildChanges` and let the caller determine the behavior.
I also thought about using `Optional`, but it doesn't seem that would add
too much value. And `Optional` will add more overhead than `null`. The original
code also does null check, so for now I'd keep the change minimum. Later we
will still have a better design and hopefully will make the code cleaner.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
