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

Ship it!


Couple of questions, just to be sure, but looks good to me.


/trunk/qpid/cpp/include/qpid/messaging/Logger.h
<https://reviews.apache.org/r/16316/#comment58472>

    virtual destructor?



/trunk/qpid/cpp/include/qpid/messaging/Logger.h
<https://reviews.apache.org/r/16316/#comment58473>

    Are there any ABI issues when relying on a virtual method in the API?


- Gordon Sim


On Dec. 17, 2013, 8:04 a.m., Andrew Stitcher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16316/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2013, 8:04 a.m.)
> 
> 
> Review request for qpid, Chug Rolke and Gordon Sim.
> 
> 
> Bugs: QPID-5415
>     https://issues.apache.org/jira/browse/QPID-5415
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Currently there is one major omission from the qpid::messaging API: That is 
> control of the internal logging used by qpid itself.
> 
> This was previously present in the qpid::client API albeit not really 
> supported or documented. Now that we have completely removed the qpid::client 
> and associated APIs from being exported for use beyond qpid itself there is 
> no way to control the logging internal to the qpid messaging libraries.
> 
> This review introduces some simple APIs which give back control of the qpid's 
> internal logging. I think there is now as much control as virtually all qpid 
> user applications are likely to need.
> 
> Please give the proposed API some consideration and let me know if there is 
> anything that an application needs that I haven't considered here.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/include/qpid/messaging/Logger.h PRE-CREATION 
>   /trunk/qpid/cpp/src/CMakeLists.txt 1551330 
>   /trunk/qpid/cpp/src/qpid/log/Logger.cpp 1551330 
>   /trunk/qpid/cpp/src/qpid/log/Options.h 1551330 
>   /trunk/qpid/cpp/src/qpid/log/Options.cpp 1551330 
>   /trunk/qpid/cpp/src/qpid/log/Selector.cpp 1551330 
>   /trunk/qpid/cpp/src/qpid/log/Statement.cpp 1551330 
>   /trunk/qpid/cpp/src/qpid/messaging/Logger.cpp PRE-CREATION 
>   /trunk/qpid/cpp/src/tests/CMakeLists.txt 1551330 
>   /trunk/qpid/cpp/src/tests/MessagingLogger.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/16316/diff/
> 
> 
> Testing
> -------
> 
> Added in unit tests which pass*
> 
> *They currently also assume a bug fix which is included in the code in this 
> review which prevents critical log messages ever being turned off (I think 
> this the correct behaviour - in the current code under some circumstances 
> they can actually be turned off)
> 
> 
> Thanks,
> 
> Andrew Stitcher
> 
>

Reply via email to