Review Request 125615: Make phonon dependency optional

[Albert Astals Cid]

> On oct. 13, 2015, 2:26 a.m., Martin Klapetek wrote:
> > As a maintainer, I would be much much more happy if it actually had a check for win/mac rather than making audio notifications optional always. Also speaking as someone who deals with knotifications bug reports.
> 
> Martin Klapetek wrote:
>     ...and I'm sure there will be one about no sound in notifications in Plasma sooner or later, which I would like to avoid.
>     
>     (pressed enter too soon)
> 
> Sune Vuorela wrote:
>     I like that things that aren't actually specific to an OS aren't marked as such, though one could also argue that it should be required unless passed -DDISABLE_PHONON or similar to cmake.
> 
> Christoph Cullmann wrote:
>     Hi, I am pragmatic, I can guard it with "optional only on mac and win" (btw., no, you can't just remove the dep there, as you can build phonon if you really want on both platforms), but I don't think that is the right way. Neither the "-DISABLE..".
>     
>     If we want to make frameworks attractive, it must be easy to build them and easy to bundle them with your application.
>     
>     With the optional dependency it is.
>     
>     If people build from source, you can anyway not avoid that they just compile a broken phonon or enable the -DISABLE.. switch.

I think -DDISABLE_PHONON or similar (-DMAKE_PHONON_OPTIONAL) is the easiest way to make it so that people that don't want the dependency can still build but that it is a conscious decision so there's fewer people that complain later at a "no sounds in Plasma" level.


- Albert


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


On oct. 12, 2015, 9:01 p.m., Christoph Cullmann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125615/
> -----------------------------------------------------------
> 
> (Updated oct. 12, 2015, 9:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Sune Vuorela.
> 
> 
> Repository: knotifications
> 
> 
> Description
> -------
> 
> Make phonon dependency optional, purely internal change, like it is done for speech.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 5a4ec83 
>   src/CMakeLists.txt 2a88b5a 
>   src/knotificationmanager.cpp 1c392b4 
> 
> Diff: https://git.reviewboard.kde.org/r/125615/diff/
> 
> 
> Testing
> -------
> 
> Still builds with (and now even without) phonon here.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20151013/ff4a06a1/attachment.html>