[GitHub] [guacamole-client] mike-jumper commented on issue #345: GUACAMOLE-234: Migrate to Apache Directory API for LDAP Extension

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

mike-jumper commented on issue #345: GUACAMOLE-234: Migrate to Apache Directory API for LDAP Extension
URL: https://github.com/apache/guacamole-client/pull/345#issuecomment-520194065
 
 
   OK, I've re-reviewed. I think everything looks fine with the exception of the comments above. Specifically:
   
   * Naming a function after its relevant noun feels funky (`referralConnection()`)
   * The documentation of the `searchHop` parameter is unclear to me.
   * One call to the logger included the exception at a level higher than debug. Exceptions should only be logged at the debug level or lower.
   * There are a few cases where `e.getMessage()` is included in the call to the logger, yet there is no `{}` to tell the logger where to put that message. I'm not sure if that will work. Even if it does, it's probably better to be explicit and use the formatting we've used elsewhere when intentionally including an exception message.
   * A single space seems to have magically materialized.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services