D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-11-23 Thread Alexander Schlarb 
ntninja added inline comments.

INLINE COMMENTS

> davidedmundson wrote in CMakeLists.txt:79
> What about this comment?

Well, it isn't actually required and it compiles just fine with both canberra 
and phonon missing.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: ntninja, kde-frameworks-devel, apol
Cc: davidedmundson, kde-frameworks-devel, apol, ltoscano, cgiboudeaux, 
michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-11-23 Thread David Edmundson 
davidedmundson added inline comments.

INLINE COMMENTS

> CMakeLists.txt:79
>  DESCRIPTION "Qt-based audio library"
> -# This is REQUIRED since you cannot tell CMake "either one of those 
> two optional ones are required"
> -TYPE REQUIRED

What about this comment?

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: ntninja, kde-frameworks-devel, apol
Cc: davidedmundson, kde-frameworks-devel, apol, ltoscano, cgiboudeaux, 
michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-11-22 Thread Christoph Feck 
cfeck set the repository for this revision to R289 KNotifications.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: ntninja, kde-frameworks-devel, apol
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-11-22 Thread Aleix Pol Gonzalez 
apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  Makes sense to me, it's even already if'd!

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: ntninja, kde-frameworks-devel, apol
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-11-22 Thread Alexander Schlarb 
ntninja updated this revision to Diff 46035.
ntninja added a comment.


  Updated to latest KF5 version and probably fixed the issue mentioned in 
review – although I have no idea what it was.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12674?vs=33575=46035

REVISION DETAIL
  https://phabricator.kde.org/D12674

AFFECTED FILES
  CMakeLists.txt

To: ntninja
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-11-22 Thread Alexander Schlarb 
ntninja added a reviewer: kde-frameworks-devel.

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: ntninja, kde-frameworks-devel
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-07-29 Thread Alexander Schlarb 
tundracomp added a comment.


  Oh, I see! This wasn't obvious to me. Is this better now?

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-07-29 Thread Alexander Schlarb 
tundracomp set the repository for this revision to R289 KNotifications.
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: kde-frameworks-devel, apol, ltoscano, cgiboudeaux, michaelh, ngraham, 
bruns, #frameworks

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-07-28 Thread Aleix Pol Gonzalez 
apol added a comment.


  You didn't specify for which repository this is.

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: apol, ltoscano, cgiboudeaux, #frameworks, michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-07-27 Thread Alexander Schlarb 
tundracomp added a comment.


  Could I get a new review on this please!?

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: ltoscano, cgiboudeaux, #frameworks, michaelh, ngraham, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-05-03 Thread Alexander Schlarb 
tundracomp added a comment.


  I used the web interface and I wasn't able to find the „Update Diff“ button 
(why not update patch or update commit?) in the right box until now. It should 
be fixed now.

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: ltoscano, cgiboudeaux, #frameworks, michaelh, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-05-03 Thread Alexander Schlarb 
tundracomp updated this revision to Diff 33575.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12674?vs=33526=33575

REVISION DETAIL
  https://phabricator.kde.org/D12674

AFFECTED FILES
  CMakeLists.txt

To: tundracomp
Cc: ltoscano, cgiboudeaux, #frameworks, michaelh, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-05-03 Thread Luigi Toscano 
ltoscano added a comment.


  If you want to update the patch on phabricator, just apply it locally with 
`arc patch Dxyzt`, amend the git commit as usual (maybe also rebase on master 
if needed), and use `arc diff` again.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: ltoscano, cgiboudeaux, #frameworks, michaelh, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-05-03 Thread Alexander Schlarb 
tundracomp added inline comments.

INLINE COMMENTS

> cgiboudeaux wrote in CMakeLists.txt:68
> there are no component, why do you use this keyword ? removing 'REQUIRED' was 
> enough

I added this keyword because Qt5TTS has it too. Anyways, how do I *update* a 
patch on this thing? Do I just use “Abandon Revision” from the drop-down below 
and start over?

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: cgiboudeaux, #frameworks, michaelh, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-05-03 Thread Christophe Giboudeaux 
cgiboudeaux added inline comments.

INLINE COMMENTS

> CMakeLists.txt:68
>  
> -find_package(Phonon4Qt5 4.6.60 REQUIRED NO_MODULE)
> +find_package(Phonon4Qt5 4.6.60 OPTIONAL_COMPONENTS NO_MODULE)
>  set_package_properties(Phonon4Qt5 PROPERTIES

there are no component, why do you use this keyword ? removing 'REQUIRED' was 
enough

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

To: tundracomp
Cc: cgiboudeaux, #frameworks, michaelh, bruns

D12674: Mark `Phonon4Qt5` dependency as optional in CMakeLists file

2018-05-02 Thread Alexander Schlarb 
tundracomp created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
tundracomp requested review of this revision.

REVISION SUMMARY
  Usage of Phonon is already optional in the source code; this commit updates 
the CMake file to reflect this. All tests pass with Phonon missing.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D12674

AFFECTED FILES
  CMakeLists.txt

To: tundracomp
Cc: #frameworks, michaelh, bruns