Review Request 129256: use cmake variables to check if QtDbus is enabled
Harald Sitter
sitter at kde.org
Tue Oct 25 08:31:40 BST 2016
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129256/#review100247
-----------------------------------------------------------
According to the referenced mail a QT_HAS_FEATURE() macro ought to be the replacement?
(that being said I think I like this better as the presented change would also work with qt4)
phonon/CMakeLists.txt (line 15)
<https://git.reviewboard.kde.org/r/129256/#comment67319>
Please keep the if ordered as it was to reduce the diff and make this easier to read at a glance without the double negation.
if (phonon_no_dbus || !qtdbus_found)
// disable
else
// enable
- Harald Sitter
On Oct. 25, 2016, 12:54 a.m., Takahiro Hashimoto wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129256/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2016, 12:54 a.m.)
>
>
> Review request for Phonon and Harald Sitter.
>
>
> Bugs: 368948
> http://bugs.kde.org/show_bug.cgi?id=368948
>
>
> Repository: phonon
>
>
> Description
> -------
>
> Fixes for BUG#368948
>
> QtCore/qfeatures.h is removed on Qt 5.8 branch for moving toward next-gen configure system[1]
> Phonon of cource can uses cmake insted of it to handle whether QtDBus is enabled or not easily.
> I think it works on both Qt4 and Qt5, but tested on Qt 5.8 branch only.
>
> [1] http://lists.qt-project.org/pipermail/development/2016-June/026350.html
>
>
> Diffs
> -----
>
> phonon/CMakeLists.txt e6dfcb7
> phonon/phononconfig_p.h.in 63f2305
>
> Diff: https://git.reviewboard.kde.org/r/129256/diff/
>
>
> Testing
> -------
>
> I've tested to build Phonon with these patterns as following. All cases is with Qt 5.8 branch. More testing by folks is very welcomed:)
>
> 1. Build with default option(Phonon DBus and QtDBus enabled) -> build succeeded with DBus support
> 2. Build with -DPHONON_NO_DBUS=TRUE cmake option -> build succeeded without DBus support
> 3. Build with default options / without QtDBus installed -> build succeeded without DBus Support.
>
> I checked the file phonon/phonon/phononconfig_p.h generaged correctly by options.
>
>
> Thanks,
>
> Takahiro Hashimoto
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20161025/4c030c0a/attachment.htm>
More information about the kde-multimedia
mailing list