Many thanks Pavel for the changes, and thanks Lance, Roger for the reviewing. Yep, let me try to handle some questions from Roger
> On 7 Aug 2019, at 11:52 PM, Roger Riggs <roger.ri...@oracle.com> wrote: > > BaseLdapServer: > > 158: Is printing the stack trace diagnostic or an error?, the exception is > not rethrown and no message clarifying > the context of the trace is printed. > Stack traces should go to the log as well or instead of stderr. It’s for diagnostic, the ldap server was designed to shadow support ldap tests, it should be a black box to ldap client unless there is intent interaction per test, so in most case the server side exception should not affect test itself, just for diagnostic. Yep, putting stack traces into log sounds good to me > LdapMessage: > > 90: plural? byte[] messages implies multiple messages, not just a buffer > for a single message. You are correct, it’s a buffer for a single message, so should be singular, getMessages() should also be rename to getMessage() to avoid confusion > > 136: Why make a copy? Is it expected to be modified? Be clear in the > javadoc about the copy. To prevent modification to original LdapMessage, yep, javadoc could be improved to make it clear > > 230: can the same trick using BigInteger (line 124) be used to extract the > length? Yes, I believe that should work since we limited the payload length presentation <= 4 bytes Regards, Chris
