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

Ship it!


Whilst this is a little messy I'm happy for it to ship although I'd be happier 
if you attend to the issues below.


/trunk/qpid/cpp/src/qpid/broker/Link.cpp
<https://reviews.apache.org/r/9055/#comment33653>

    There appears to be no substitute for this test perhaps should be:
        if (state == STATE_CLOSING) return;
    
    Perhaps it doesn't matter except as a shortcut return.



/trunk/qpid/cpp/src/qpid/broker/Link.cpp
<https://reviews.apache.org/r/9055/#comment33654>

    perhaps the entire "if (state ==" chain should be replaced with a "switch 
... case" type of construct to make the state machine design more plain.



/trunk/qpid/cpp/src/qpid/broker/Link.cpp
<https://reviews.apache.org/r/9055/#comment33652>

    I think this change has no effect - that is the new code has precisely the 
same effect as the old.


- Andrew Stitcher


On Jan. 22, 2013, 6:25 p.m., Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9055/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2013, 6:25 p.m.)
> 
> 
> Review request for qpid, Andrew Stitcher and Ted Ross.
> 
> 
> Description
> -------
> 
> Fixes a bug that prevents deletion of a Link when the corresponding 
> connection is not in the active state.
> 
> 
> This addresses bug qpid-4546.
>     https://issues.apache.org/jira/browse/qpid-4546
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/Link.h 1436517 
>   /trunk/qpid/cpp/src/qpid/broker/Link.cpp 1436517 
> 
> Diff: https://reviews.apache.org/r/9055/diff/
> 
> 
> Testing
> -------
> 
> Federation unit tests + tested deleting links that were disconnected and in 
> connection state.
> 
> 
> Thanks,
> 
> Kenneth Giusti
> 
>

Reply via email to