Re: Review Request 129256: use cmake variables to check if QtDbus is enabled

Harald Sitter <[email protected]>
Newsgroups gmane.comp.kde.users.multimedia
Message-ID <[email protected]>
-----------------------------------------------------------
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
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.