<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/4898/">http://reviewboard.kde.org/r/4898/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 4th, 2010, 4:08 p.m., <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
<p>On August 4th, 2010, 8:10 p.m., <b>Marc Mutz</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 :)</pre>
</blockquote>
<p>On August 4th, 2010, 9 p.m., <b>Aurélien Gâteau</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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?)</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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 :)</pre>
<br />
<p>- Marc</p>
<br />
<p>On August 4th, 2010, 9:37 a.m., Marc Mutz wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Marc Mutz.</div>
<p style="color: grey;"><i>Updated 2010-08-04 09:37:48</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Make DBusMenuQt optional.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Compiles w/o DBusMenuQt present.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/KDE/4.5/kdelibs/CMakeLists.txt <span style="color: grey">(1159039)</span></li>
<li>/branches/KDE/4.5/kdelibs/ConfigureChecks.cmake <span style="color: grey">(1159039)</span></li>
<li>/branches/KDE/4.5/kdelibs/config.h.cmake <span style="color: grey">(1159039)</span></li>
<li>/branches/KDE/4.5/kdelibs/kdeui/CMakeLists.txt <span style="color: grey">(1159039)</span></li>
<li>/branches/KDE/4.5/kdelibs/kdeui/notifications/kstatusnotifieritem.cpp <span style="color: grey">(1159039)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4898/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>