[GitHub] sijie commented on a change in pull request #1252: Set `ENABLE_DIGEST_TYPE_AUTODETECTION` to true as default value

[GitBox <gi...@apache.org>]

sijie commented on a change in pull request #1252: Set `ENABLE_DIGEST_TYPE_AUTODETECTION` to true as default value
URL: https://github.com/apache/bookkeeper/pull/1252#discussion_r174934565
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
 ##########
 @@ -741,7 +741,8 @@ ZooKeeper getZkHandle() {
         return ((ZKMetadataClientDriver) metadataDriver).getZk();
     }
 
-    protected ClientConfiguration getConf() {
+    @VisibleForTesting
+    public ClientConfiguration getConf() {
 
 Review comment:
   > This is not good.
   
   forget "VisibleForTesting" annotation, it is very typical to have `getConf` api in a client to retrieve the configuration used by a given client. which I don't think make `getConf` is considered "not good".
   
   I am adding "VisibleForTesting" because this only exists in old bookkeeper api. Ideally the new api should provide `getConf` as well.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services