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.or
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?
REPOSITOR
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
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, mi
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&id=46035
REVISION DETAIL
https
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
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
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, apo
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
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
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,
tundracomp updated this revision to Diff 33575.
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D12674?vs=33526&id=33575
REVISION DETAIL
https://phabricator.kde.org/D12674
AFFECTED FILES
CMakeLists.txt
To: tundracomp
Cc: ltoscano, cgiboudeaux, #frameworks, michaelh, bruns
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
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 “A
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 ?
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
16 matches
Mail list logo