<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="https://git.reviewboard.kde.org/r/115218/">https://git.reviewboard.kde.org/r/115218/</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 22nd, 2014, 9:27 p.m. UTC, <b>Valentin Rusu</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/115218/diff/1/?file=235142#file235142line100" style="color: black; font-weight: bold; text-decoration: underline;">src/api/KWallet/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">100</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nb">install</span><span class="p">(</span><span class="s">FILES</span> <span class="o">${</span><span class="nv">kwallet_xml</span><span class="o">}</span> <span class="s">DESTINATION</span> <span class="o">${</span><span class="nv">DBUS_INTERFACES_INSTALL_DIR</span><span class="o">}</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">100</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="nb">install</span><span class="p">(</span><span class="s">FILES</span> <span class="o">${</span><span class="nv">kwallet_xml</span><span class="o">}</span> <span class="s">DESTINATION</span> <span class="o">${</span><span class="nv">DBUS_INTERFACES_INSTALL_DIR</span><span class="o">}</span><span class="hl"> </span><span class="s"><span class="hl">RENAME</span></span><span class="hl"> </span><span class="s"><span class="hl">kf5_org.kde.KWallet.xml</span></span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<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 interface will be strictly the same. New features will be added to secret service. That's why I didn't consider changing it's name. And, btw, changing it's name will make users think that changes have been (or will be) done to this API.</pre>
</blockquote>
<p>On January 22nd, 2014, 9:43 p.m. UTC, <b>Hrvoje Senjan</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;">@Valentin,
interface name is now org.kde.kwalletd5, as per your last change in api/KWallet/kwallet.cpp & runtime/kwalletd/kwalletd.cpp.</pre>
</blockquote>
<p>On January 23rd, 2014, 12:22 p.m. UTC, <b>Jonathan Riddell</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;">Yes you changed the interface which you say is unnecessary. My patch is just a change of filename to allow co-installability with kdelibs4 but the dbus interface name isn't changed.
</pre>
</blockquote>
<p>On January 23rd, 2014, 9:43 p.m. UTC, <b>Valentin Rusu</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;">Considering that, with KDE4:
kwalletd
|
--- exposes org.kde.kwalletd
|
----- implements interface org.kde.KWallet.xml
With KF5:
kwalletd5 (binary, must have different name to coinstall in same bin directory)
|
--- exposes org.kde.kwalletd5 (dbus object name, must have different name to be able to be launched on the same dbus session)
|
----- implements interface org.kde.KWallet.xml the same as KDE4, installation directory is the same, but if replaced there's no conflict (or am I wrong?)
I think that there is no need to do a second copy of og.kde.KWallet.xml
What do you think about this?
</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">>exposes org.kde.kwalletd5 (dbus object name, must have different name to be able to be launched on the same dbus session)
ApplicationName still has kwalletd, so two daemons (kwalletd & kwalletd5) atm cannot be started. i guess that was just omission in your change? ;-)
>but if replaced there's no conflict
how do you mean? we cannot ship 2 packages providing the same filename and not make the conflict. that is the point of this request =)
(yes, it could be confusing, but so far we found this to be the 'best' approach - if interface is left to be named same as gen 4.x. maybe to add a comment above the install() section?)</pre>
<br />
<p>- Hrvoje</p>
<br />
<p>On January 22nd, 2014, 11:23 a.m. UTC, Jonathan Riddell wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks and Valentin Rusu.</div>
<div>By Jonathan Riddell.</div>
<p style="color: grey;"><i>Updated Jan. 22, 2014, 11:23 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwallet-framework
</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;">Rename kwallet dbus interface file on install so it does not clash with equivalent file from kdelibs4. The dbus interface remains the same to keep compatibility with kdelibs4 kwallet.
However you seem to have renamed the interface in places in the code in runtime/, will it be incompatible with the version from kdelibs4?
</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>src/api/KWallet/CMakeLists.txt <span style="color: grey">(d0d5a3d)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115218/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>