qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rafael Schloming" <...@apache.org>
Subject Re: Review Request 22021: [PROTON-589] Changes proposed to Messenger interface and supporting interfaces
Date Mon, 02 Jun 2014 20:49:29 GMT

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



http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/FileDescriptor.java
<https://reviews.apache.org/r/22021/#comment78962>

    It's a little bit odd to have an interface named FileDescriptor in Java. We could address
that by trying to come up with another name for it, but I wonder if it is actually needed.
I think most of what it provides is already provided by the Transport interface. I would be
curious to see if we could replace this and use the existing Transport interface instead.



http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/Listener.java
<https://reviews.apache.org/r/22021/#comment78961>

    I notice you use getHost() on Selectable and getHostName() here. Assuming there isn't
a reason I'm missing for the difference we should probably be consistent. (My vote would be
getHost() as this can probably return things like "1.2.3.4" which is not strictly speaking
a name.)



http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/Selectable.java
<https://reviews.apache.org/r/22021/#comment78966>

    The naming here could be more coherent. Selectable, FileDescriptor, and ConnectionEventHandler
seem to all be using different terms to refer to similar/the same thing.


- Rafael Schloming


On May 29, 2014, 3:07 p.m., rajith attapattu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22021/
> -----------------------------------------------------------
> 
> (Updated May 29, 2014, 3:07 p.m.)
> 
> 
> Review request for qpid and Rafael Schloming.
> 
> 
> Bugs: PROTON-589
>     https://issues.apache.org/jira/browse/PROTON-589
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Passive mode allows the file descriptors for messenger to be serviced by an external
loop.
> This patch contains changes proposed to Messenger interface and supporting interfaces.
> 
> Please note my working copy contains some very slight changes to the supporting interfaces.
> But the core direction taken is the same.
> 
> 
> Diffs
> -----
> 
>   http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/ConnectionEventHandler.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/FileDescriptor.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/Listener.java
PRE-CREATION 
>   http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/Messenger.java
1598310 
>   http://svn.apache.org/repos/asf/qpid/proton/trunk/proton-j/src/main/java/org/apache/qpid/proton/messenger/Selectable.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22021/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> rajith attapattu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message