----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35658/#review88815 -----------------------------------------------------------
proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java (line 100) <https://reviews.apache.org/r/35658/#comment141403> We don't tend to namespace things like this in Java as the classes do it already. I'd suggest calling it CHANNEL_MAX_LIMIT or something like that to convey its purpose. statics are usually declared at the top. I would move it up to beside BUFFER_RELEASE_THRESHOLD. Finally, I would suggest making it private rather than public, as nothing else looks to be using it, and likely won't. proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java (line 213) <https://reviews.apache.org/r/35658/#comment141408> I would tend to throw an IllegalArgumentException if validating the value is suitable, rather than silently bounding the given value. I guess the comment from Ken about proton-c return values is possibly why you went this way. If we do go this way though, shouldn't we handle negative values similarly? tests/python/proton_tests/engine.py (line 259) <https://reviews.apache.org/r/35658/#comment141409> whitespace - Robbie Gemmell On June 22, 2015, 5:04 p.m., michael goulish wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35658/ > ----------------------------------------------------------- > > (Updated June 22, 2015, 5:04 p.m.) > > > Review request for qpid, Kenneth Giusti, Robbie Gemmell, and Ted Ross. > > > Repository: qpid-proton-git > > > Description > ------- > > Fix the java tests that my previous checkin broke. Also implement the logic > (in the Java impl) to prevent channel_max from being changed by the > application after the OPEN frame has been sent (because at that time we tell > the remote peer what our channel_max is). > > > Diffs > ----- > > > proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java > a5c8ba9 > tests/python/proton_tests/engine.py e3258a2 > > Diff: https://reviews.apache.org/r/35658/diff/ > > > Testing > ------- > > ctest -VV > > > Thanks, > > michael goulish > >
