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: [email protected] With regards, Apache Git Services
