> 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.
>
> Aaron Seigo wrote:
> 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. :)
regarding this: "there once was a policy to not require new (hard) dependencies
in new releases. Seems to have slipped off the radar, though."
yes, i remember that as well (for kdelibs, anyways) and think we should get
that back on track. unless there are any objections, i will add some text to
this affect to the library policy page here so that we can make it part of our
institutional memory, aka "the policies wiki":
http://techbase.kde.org/Policies/Library_Code_Policy
- 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
