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