<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/126691/">https://git.reviewboard.kde.org/r/126691/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 10th, 2016, 2:06 p.m. CET, <b>David Edmundson</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That sounds like you're saying Qt has broken API.
If that's true, they'll want to know about it.</p></pre>
</blockquote>
<p>On February 7th, 2016, 11:30 p.m. CET, <b>David Kahles</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I talked to them (see https://codereview.qt-project.org/#/c/144823/) and it seems like we shouldn't have done it like it was before, it worked by pure luck.
According to them, this change will do the right thing, but additional, we shouldn't inherit containers at all.
(I think the problem is, that QT containers don't have a virtual destructor)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Btw., libdbusmenu-qt suffers from the same problem, but I'm not sure whether this is our responsibility (I'm confused because there's kind of a fork in plasma).</p></pre>
</blockquote>
<p>On February 7th, 2016, 11:33 p.m. CET, <b>David Edmundson</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Right so:
- libdbusmenu-qt is on launchpad somewhere (though not a canonical project or anything)
- I /needed/ a patch that fixed a crash but it had to break API, so I copied a tiny part of it (client only) into our code, whilst the original is in review. AFAIK it still is.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ok thanks, that's what i've assumed.
@D?vis Mos?ns: It would be great if you could additional try to get this change upstream (https://launchpad.net/libdbusmenu-qt)</p></pre>
<br />
<p>- David</p>
<br />
<p>On January 10th, 2016, 2:59 a.m. CET, Dāvis Mosāns wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Dāvis Mosāns.</div>
<p style="color: grey;"><i>Updated Jan. 10, 2016, 2:59 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Add DBusMenuShortcut type overload for QDBusArgument.
I don't know if there's a better way to fix this than this kinda code duplication.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This is needed because otherwise it wouldn't compile with latest Qt dev branch. It's probably because QList<T> overload was removed in http://code.qt.io/cgit/qt/qtbase.git/commit/src/dbus/qdbusargument.h?h=dev&id=5f542f3cca13f2da58b82aee2efbaffefeee00a7</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">and Container<T> doesn't work...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">/usr/include/QtDBus/qdbusargument.h:244:29: note: candidate: template<template<class> class Container, class T> const QDBusArgument& operator>>(const QDBusArgument&, Container<T>&)
inline const QDBusArgument &operator>>(const QDBusArgument &arg, Container<T> &list)
^
/usr/include/QtDBus/qdbusargument.h:244:29: note: template argument deduction/substitution failed:
/mnt/KDE/kde/workspace/plasma-workspace/dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp:261:16: note: can't deduce a template for ‘Container<T>’ from non-template type ‘DBusMenuShortcut’
arg >> dmShortcut;
^
In file included from /usr/include/QtDBus/qdbuspendingreply.h:39:0,
from /usr/include/QtDBus/qdbusreply.h:44,
from /usr/include/QtDBus/QDBusReply:1,
from /mnt/KDE/kde/workspace/plasma-workspace/dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp:27</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Compiles</p></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>dataengines/statusnotifieritem/libdbusmenuqt/dbusmenutypes_p.h <span style="color: grey">(4950a22279c09fb93c68fe3d38ff600279e856ca)</span></li>
<li>dataengines/statusnotifieritem/libdbusmenuqt/dbusmenutypes_p.cpp <span style="color: grey">(e98c4b93bc8532367ee96138ce72a54f44ac05ca)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126691/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>