D21660: change audio dep logic wrt win32

brute4s99 updated this revision to Diff 61639. brute4s99 added a comment. no more dbus on Windows build of KNotifications REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21660?vs=61422&id=61639 BRANCH arcpatch-D21660_1 REVISION DETAIL https://ph

D21660: change audio dep logic wrt win32

brute4s99 added inline comments. INLINE COMMENTS > nicolasfella wrote in CMakeLists.txt:38 > But this is KNotifications, not KDE Connect. So we need DBus for > KNotifications on Windows? ah, yeah we could bypass DBus on Windows in KNotifications REPOSITORY R289 KNotifications BRANCH arcpa

D21660: change audio dep logic wrt win32

nicolasfella added inline comments. INLINE COMMENTS > brute4s99 wrote in CMakeLists.txt:38 > Update: nope. I think it's still use for the dbus interfaces. I confirmed by > ending the `dbus-daemon.exe` process by hand to see it. Turns out, dbus is > quite important for KDE Connect regardless of

D21660: change audio dep logic wrt win32

brute4s99 added inline comments. INLINE COMMENTS > brute4s99 wrote in CMakeLists.txt:38 > I'm looking into the DBus problem, we currently use DBus for the system tray > icon on Windows. Once that is resolved, I should be able to completely remove > DBus from the Windows build Update: nope. I t

D21660: change audio dep logic wrt win32

brute4s99 added inline comments. INLINE COMMENTS > nicolasfella wrote in CMakeLists.txt:38 > Yes. It is neither related to Windows nor audio. What you can to is to add > Windows to the condition to not look for DBus on Windows, but please do that > in a separate patch. Or move the find call for

D21660: change audio dep logic wrt win32

nicolasfella accepted this revision. This revision is now accepted and ready to land. REPOSITORY R289 KNotifications BRANCH arcpatch-D21660 REVISION DETAIL https://phabricator.kde.org/D21660 To: brute4s99, broulik, nicolasfella Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGa

D21660: change audio dep logic wrt win32

brute4s99 updated this revision to Diff 61422. brute4s99 added a comment. updated REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21660?vs=61421&id=61422 BRANCH arcpatch-D21660 REVISION DETAIL https://phabricator.kde.org/D21660 AFFECTED FILES

D21660: change audio dep logic wrt win32

nicolasfella added inline comments. INLINE COMMENTS > brute4s99 wrote in CMakeLists.txt:38 > I was trying to simplify code there by removing `NOT`. Should I remove this > change? Yes. It is neither related to Windows nor audio. What you can to is to add Windows to the condition to not look for

D21660: change audio dep logic wrt win32

brute4s99 added inline comments. INLINE COMMENTS > nicolasfella wrote in CMakeLists.txt:38 > This seems to be entirely urelated to audio? I was trying to simplify code there by removing `NOT`. Should I remove this change? REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.

D21660: change audio dep logic wrt win32

brute4s99 edited the summary of this revision. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21660 To: brute4s99, broulik, nicolasfella Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21660: change audio dep logic wrt win32

brute4s99 updated this revision to Diff 61421. brute4s99 added a comment. rebased REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21660?vs=59551&id=61421 BRANCH arcpatch-D21660 REVISION DETAIL https://phabricator.kde.org/D21660 AFFECTED FILES

D21660: change audio dep logic wrt win32

nicolasfella requested changes to this revision. nicolasfella added a comment. This revision now requires changes to proceed. This needs rebasing since I recently changed the audio logic for Android INLINE COMMENTS > CMakeLists.txt:38 > find_package(Qt5 ${REQUIRED_QT_VERSION} CONFIG REQUIRED

D21660: change audio dep logic wrt win32

brute4s99 added a comment. may I land this? REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21660 To: brute4s99, broulik Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21660: change audio dep logic wrt win32

brute4s99 marked 2 inline comments as done. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21660 To: brute4s99, broulik Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21660: change audio dep logic wrt win32

brute4s99 added a comment. In D21660#481421 , @bcooksley wrote: > Where possible D-Bus should be avoided on Windows. Thanks for taking a look, @bcooksley! We already use dbus-daemon.exe for a lot of talking with the notifications and (I

D21660: change audio dep logic wrt win32

bcooksley added a comment. With regards to Windows, please note that any unit test which depends on calls that involve D-Bus on the CI system will likely lead to that test hanging because dbus-daemon is not launched by the CI system. Where possible D-Bus should be avoided on Windows. REPOS

D21660: change audio dep logic wrt win32

brute4s99 marked an inline comment as done. brute4s99 added inline comments. INLINE COMMENTS > nicolasfella wrote in CMakeLists.txt:42 > We don't need DBus on Windows, do we? we don't, I guess, but dbus-daemon.exe still runs in the background so I can't say. The functionality doesn't seem to h

D21660: change audio dep logic wrt win32

brute4s99 retitled this revision from "simplify conditions and remove audio dependency for win32" to "change audio dep logic wrt win32". REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21660 To: brute4s99, broulik Cc: apol, nicolasfella, kde-frameworks-devel, LeG