> On 2010-08-04 16:08:34, Aurélien Gâteau wrote: > > The change looks OK, but I am wondering why you do not want to link with > > DBusMenuQt. Having (very) different code paths is potentially a source of > > trouble for maintenance. > > Marc Mutz wrote: > Having that kind of library as a hard dependency of KDE is just gross. > Have a look at what other libraries kdelibs depends on, and how many of them > are actually required. This library is used for a one-liner, literally. It > shouldn't be required, esp. as there doesn't seem to be any release yet. > Symptomatic post: > http://www.mail-archive.com/kde-buildsystem%40kde.org/msg05473.html > IIRC, there once was a policy to not require new (hard) dependencies in > new releases. Seems to have slipped off the radar, though. > > On top of that, on mobile, every dependency we don't have to have on the > phone is a good dependency :) > > Aurélien Gâteau wrote: > I don't see how depending on such a library is "gross", can you explain a > bit more? > > Regarding the message you refer to, I agree the dependency could have > been introduced in a much nicer way and I acknowledged this on my blog ( > http://agateau.wordpress.com/2010/04/29/about-dbusmenu-holidays-and-other-hacks/ > ). I thought that episode was behind us now. > > Regarding the lack of releases, here is the url where you can download > release tarballs: https://launchpad.net/libdbusmenu-qt/+download . There is a > link to the Launchpad page in the FindDBusMenuQt.cmake file. > > When DBusMenuQt is not used, the application is in charge of showing the > menu, not the system tray. This is what I mean with a very different code > path. I won't oppose against the change, though. I think the decision should > be up to Marco Martin (can you add him to the "People" field?) > > Marc Mutz wrote: > > I don't see how depending on such a library is "gross", can you explain > a bit more? > > Sorry, it's not the depending on it per se. I find it gross that > DBusMenuQt is required when it's only used in a single file in the > notification subsystem of kdeui whereas not even OpenSSL is a required > dependency, without which you can't even use half the world's web sites. > > > Regarding the lack of releases, here is the url where you can download > release tarballs: > > https://launchpad.net/libdbusmenu-qt/+download . > > There is a link to the Launchpad page in the FindDBusMenuQt.cmake file. > > All that the macro_log_feature() in kdelibs/CMakeLists.txt told me was to > git-clone something. I didn't occur to me to look in FindDBusMenuQt.cmake for > a link to releases :) > > Marc Mutz wrote: > Oh, and what does libdbusmenu-qt do on Windows, WinCE, and OSA X? > > Aurélien Gâteau wrote: > > I find it gross that DBusMenuQt is required when it's only used in a > single file in the notification subsystem of kdeui > > One half of DBusMenuQt is used in kdeui, the other half is used in the > systemtray applet (which you would need to patch as well). > > > All that the macro_log_feature() in kdelibs/CMakeLists.txt told me was > to git-clone something. I didn't occur to me to look in FindDBusMenuQt.cmake > for a link to releases :) > > The url is an argument to find_package_handle_standard_args(), not a > comment. I assumed it would show up when you ran cmake and it failed to find > the library. I may be wrong. > > > Oh, and what does libdbusmenu-qt do on Windows, WinCE, and OS X? > > Honestly I don't know, but I know the KDE Windows people have contributed > patches to ensure it builds, so I guess it works. > > Volker Krause wrote: > It even builds on WinCE, but there is definitely no host using it, so > it's just using rare resources without any benefit.
on WinCE / Windows Mobile and MacOS, there isn't a host. on MS Windows, there are people who use Plasma Desktop as their shell (i know because we get bug reports from them :P), and at that point there is a host on that platform (though the system tray needs some additional porting on the Windows platform as it doesn't yet support the native icons) ... probably more details than needed. :) - Aaron ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/4898/#review6793 ----------------------------------------------------------- On 2010-08-06 12:34:16, Marc Mutz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/4898/ > ----------------------------------------------------------- > > (Updated 2010-08-06 12:34:16) > > > Review request for kdelibs, Plasma and Marco Martin. > > > Summary > ------- > > Make DBusMenuQt optional. > > > Diffs > ----- > > /branches/KDE/4.5/kdebase/workspace/CMakeLists.txt 1159836 > /branches/KDE/4.5/kdebase/workspace/ConfigureChecks.cmake 1159836 > /branches/KDE/4.5/kdebase/workspace/config-workspace.h.cmake 1159836 > > /branches/KDE/4.5/kdebase/workspace/plasma/generic/dataengines/statusnotifieritem/CMakeLists.txt > 1159836 > > /branches/KDE/4.5/kdebase/workspace/plasma/generic/dataengines/statusnotifieritem/statusnotifieritemsource.cpp > 1159836 > /branches/KDE/4.5/kdelibs/CMakeLists.txt 1159039 > /branches/KDE/4.5/kdelibs/ConfigureChecks.cmake 1159039 > /branches/KDE/4.5/kdelibs/config.h.cmake 1159039 > /branches/KDE/4.5/kdelibs/kdeui/CMakeLists.txt 1159039 > /branches/KDE/4.5/kdelibs/kdeui/notifications/kstatusnotifieritem.cpp > 1159039 > > Diff: http://reviewboard.kde.org/r/4898/diff > > > Testing > ------- > > Compiles w/o DBusMenuQt present. > > > Thanks, > > Marc > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
