You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/06/03 21:25:12 UTC
[GitHub] [helix] jiajunwang commented on a change in pull request #1033: Add path existing check for customized state
jiajunwang commented on a change in pull request #1033:
URL: https://github.com/apache/helix/pull/1033#discussion_r434866114
##########
File path: helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
##########
@@ -1258,12 +1258,12 @@ protected void checkLiveInstancesObservation(List<LiveInstance> liveInstances,
if (lastInstances == null || !lastInstances.containsKey(instance)) {
try {
manager.addCustomizedStateRootChangeListener(this, instance);
- logger.info(manager.getInstanceName() + " added customized root change listener for"
- + " " + instance
- + ", listener: " + this);
+ logger.info(manager.getInstanceName() + " added root path listener for customized "
+ + "state change for" + " " + instance + ", listener: " + this);
} catch (Exception e) {
- logger.error("Fail to add customized root change listener for instance: " + instance,
- e);
+ logger.error(
+ "Fail to add root path listener for customized state change" + "for instance: "
Review comment:
Why use 2 separate sections for the constant string contents?
##########
File path: helix-core/src/main/java/org/apache/helix/controller/GenericHelixController.java
##########
@@ -1258,12 +1258,12 @@ protected void checkLiveInstancesObservation(List<LiveInstance> liveInstances,
if (lastInstances == null || !lastInstances.containsKey(instance)) {
try {
manager.addCustomizedStateRootChangeListener(this, instance);
- logger.info(manager.getInstanceName() + " added customized root change listener for"
- + " " + instance
- + ", listener: " + this);
+ logger.info(manager.getInstanceName() + " added root path listener for customized "
Review comment:
`+ "state change for" + " "`
Can we please refine the format as well.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org