----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46879/#review131479 -----------------------------------------------------------
proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java (line 468) <https://reviews.apache.org/r/46879/#comment195486> There should be a null check on the hostname before the test of whether it isEmpty(). If this isn't throwing a NullPointerException then that suggests it is the actually empty string, which I'd say this change needs to address in order to be complete. An equivalent of the new tests that are being added for the C reactor would highlight if thats the case. - Robbie Gemmell On May 1, 2016, 11:54 p.m., Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46879/ > ----------------------------------------------------------- > > (Updated May 1, 2016, 11:54 p.m.) > > > Review request for qpid, Chug Rolke, Justin Ross, and Robbie Gemmell. > > > Bugs: PROTON-1181 > https://issues.apache.org/jira/browse/PROTON-1181 > > > Repository: qpid-proton-git > > > Description > ------- > > See linked JIRA > > > Diffs > ----- > > proton-c/src/engine/engine.c c620101 > proton-c/src/reactor/connection.c 336d1f1 > proton-c/src/tests/reactor.c 9564569 > proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/ReactorImpl.java > d13cfbe > tests/python/proton_tests/reactor.py 6ee107d > > Diff: https://reviews.apache.org/r/46879/diff/ > > > Testing > ------- > > New unit tests added. > > > Thanks, > > Kenneth Giusti > >
