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

> On 10月 25, 2016, 4:31 p.m., Harald Sitter wrote:
> > 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)

Maybe it is. I agree with you my change is better for phonon at present because of minimizing changes.


> On 10月 25, 2016, 4:31 p.m., Harald Sitter wrote:
> > phonon/CMakeLists.txt, line 15
> > <https://git.reviewboard.kde.org/r/129256/diff/1/?file=482941#file482941line15>
> >
> >     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

Oh...exactly. I don't know why I didn't do so. Thank you:)


- Takahiro


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


On 10月 25, 2016, 9:54 a.m., Takahiro Hashimoto wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129256/
> -----------------------------------------------------------
> 
> (Updated 10月 25, 2016, 9: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
> 
>