> On 2012-02-28 17:27:10, Andrew Stitcher wrote:
> > branches/asyncstore/cpp/src/qpid/broker/AsyncInterface.h, line 14
> > <https://reviews.apache.org/r/4085/diff/1/?file=86244#file86244line14>
> >
> >     This is a strange looking sequence diagram, probably better to use a 
> > more standard type of sequence diagram

I did not intend this to be a sequence diagram, just the sort of thing I would 
put on a whiteboard to explain the idea.


> On 2012-02-28 17:27:10, Andrew Stitcher wrote:
> > branches/asyncstore/cpp/src/qpid/broker/AsyncInterface.h, line 87
> > <https://reviews.apache.org/r/4085/diff/1/?file=86244#file86244line87>
> >
> >     See below - these are also Handles and so should encapsulate the ref 
> > counting not expose it.

As in the first propsal, this was left in place to illustrate the need for ref 
counting, but is to be replaced by a mechanism that will hide the boost bits.


> On 2012-02-28 17:27:10, Andrew Stitcher wrote:
> > branches/asyncstore/cpp/src/qpid/broker/AsyncInterface.h, line 107
> > <https://reviews.apache.org/r/4085/diff/1/?file=86244#file86244line107>
> >
> >     Don't do construction/destruction inline like this in API code.
> >     
> >     Do it a the .cpp

Agreed. While this is a propsal, it is (a little) helpful to keep it compact 
and in one file.


> On 2012-02-28 17:27:10, Andrew Stitcher wrote:
> > branches/asyncstore/cpp/src/qpid/broker/AsyncInterface.h, line 160
> > <https://reviews.apache.org/r/4085/diff/1/?file=86244#file86244line160>
> >
> >     This comment does no correspond to the interface below.

Yep, I forgot to update these. But the basic idea is still plain.


> On 2012-02-28 17:27:10, Andrew Stitcher wrote:
> > branches/asyncstore/cpp/src/qpid/broker/AsyncInterface.h, line 183
> > <https://reviews.apache.org/r/4085/diff/1/?file=86244#file86244line183>
> >
> >     I don't understand what "next...()" means
> >     in the name of the Handle factory methods.
> >     
> >     The interface is creating Handles so create...Handle(...) or 
> > make...Handle() would be more conventional.

Agreed. These are a holdover from the IdTokens of the first interface idea in 
whcih identity was implicit in the handle. The next...() calls would create a 
new token with a new identity.


- Kim


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


On 2012-02-28 16:29:47, Kim van der Riet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4085/
> -----------------------------------------------------------
> 
> (Updated 2012-02-28 16:29:47)
> 
> 
> Review request for qpid, Andrew Stitcher, Alan Conway, Gordon Sim, Kenneth 
> Giusti, and Ted Ross.
> 
> 
> Summary
> -------
> 
> This is the second design draft for an asyncronous store interface after 
> feedback from the previous review was received. (See QPID-3858 
> https://issues.apache.org/jira/browse/QPID-3858 for an overview.)
> 
> Much of the commentary is in the source. Comments are welcome.
> 
> 
> Diffs
> -----
> 
>   branches/asyncstore/cpp/src/CMakeLists.txt 1294116 
>   branches/asyncstore/cpp/src/Makefile.am 1294116 
>   branches/asyncstore/cpp/src/qpid/broker/AsyncInterface.h PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/4085/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kim
> 
>

Reply via email to