> On June 22, 2015, 5:48 p.m., Robbie Gemmell wrote:
> > proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java,
> >  line 100
> > <https://reviews.apache.org/r/35658/diff/1/?file=988465#file988465line100>
> >
> >     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.

fixed


> On June 22, 2015, 5:48 p.m., Robbie Gemmell wrote:
> > proton-j/src/main/java/org/apache/qpid/proton/engine/impl/TransportImpl.java,
> >  line 213
> > <https://reviews.apache.org/r/35658/diff/1/?file=988465#file988465line213>
> >
> >     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?

fixed


> On June 22, 2015, 5:48 p.m., Robbie Gemmell wrote:
> > tests/python/proton_tests/engine.py, line 261
> > <https://reviews.apache.org/r/35658/diff/1/?file=988466#file988466line261>
> >
> >     whitespace

fixed.   saved whitespace characters in file for possible use later.


- michael


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35658/#review88815
-----------------------------------------------------------


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

Reply via email to