Review Request 119085: Added disabling/enabling logging for entity

Dan Vrátil dvratil at redhat.com
Mon Jul 7 11:43:54 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119085/#review61803
-----------------------------------------------------------


The version-check approach is OK, I would just propose using a more generic approach (see below).


TelepathyLoggerQt4/log-manager.h
<https://git.reviewboard.kde.org/r/119085/#comment43012>

    Could you please use this function instead? It can be use more generically than having to define __TPL_VERSION_DISABLE_SOMETHING__ for each feature we need a version check for and it's also cleaner when used in code
    
        #ifndef TPLQT_TPL_IS_VERSION
        #define TPLQT_TPL_IS_VERSION(maj, min, patch) \
           (TPLQT_TPL_VERSION_MAJOR >= maj) \
        || (TPLQT_TPL_VERSION_MAJOR == maj && TPLQT_TPL_VERSION_MINOR >= min) \
        || (TPLQT_TPL_VERSION_MAJOR == maj && TPLQT_TPL_VERSION_MINOR == min && TPLQT_TPL_VERSION_PATCH >= patch)
        #endif



TelepathyLoggerQt4/log-manager.h
<https://git.reviewboard.kde.org/r/119085/#comment43013>

    With the function I outlined above, this will change to
    
        #ifdef TPLQT_TPL_IS_VERSION(0, 9, 0)



CMakeLists.txt
<https://git.reviewboard.kde.org/r/119085/#comment43014>

    I think we should use the TPLQT prefix instead of underscores, so the definitions would look like `TPLQT_TPL_VERSION_MAJOR` etc


- Dan Vrátil


On July 3, 2014, 10:06 p.m., Marcin Ziemiński wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119085/
> -----------------------------------------------------------
> 
> (Updated July 3, 2014, 10:06 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: telepathy-logger-qt
> 
> 
> Description
> -------
> 
> Added simple wrappers of functions available in the master branch of telepathy-logger
> 
> 
> Diffs
> -----
> 
>   cmake/modules/FindTelepathyLogger.cmake 19de2e3 
>   TelepathyLoggerQt4/log-manager.cpp 7aadc4d 
>   CMakeLists.txt db1da08 
>   TelepathyLoggerQt4/log-manager.h 60a0e8c 
> 
> Diff: https://git.reviewboard.kde.org/r/119085/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marcin Ziemiński
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140707/fc24206e/attachment-0001.html>


More information about the KDE-Telepathy mailing list