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