<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/130188/">https://git.reviewboard.kde.org/r/130188/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 28th, 2017, 8:15 p.m. UTC, <b>David Edmundson</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/130188/diff/1/?file=498227#file498227line211" style="color: black; font-weight: bold; text-decoration: underline;">KTp/Models/accounts-list-model.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">201</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">call</span><span class="p">.</span><span class="n">waitForFinished</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">why do we have to block?</p></pre>
</blockquote>
<p>On August 1st, 2017, 7 p.m. UTC, <b>James Smith</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;">The call is async so the error value can be non-valid.</p></pre>
</blockquote>
<p>On August 17th, 2017, 4:54 p.m. UTC, <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;">Yes I can read what the code is doing, but why do we need to wait for that?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We should be able to return true here, the model (should) get a dataChanged when it updates or not. Handle it outside this method.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We're not merging any blocking calls in KTp.</p></pre>
</blockquote>
<p>On August 17th, 2017, 6:13 p.m. UTC, <b>James Smith</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;">waitForFinished() blocks only the calling thread until the call's reply is returned and processed. This is the proper way to obtain valid return values from async calls, there is no way to guarantee a timely reply to the caller due to possible RPC mechanism contention and the non-blocking nature of async calls.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Non-async calls block the RPC mechanism and as a consequence the caller, providing timely replies for the caller but potentially DoS'ing the called interface.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Providing a valid return value is useful if the consumer needs to set the presence manually, for example the kded module is not available.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Read my second sentence.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also the entirity of TelepathyQt is async DBus calls, if we can call requestPresence on a connection manager in a proper correct way, we can make a similar call to KDED in a correct way.</p></pre>
<br />
<p>- David</p>
<br />
<p>On July 28th, 2017, 6:08 p.m. UTC, James Smith 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 Telepathy.</div>
<div>By James Smith.</div>
<p style="color: grey;"><i>Updated July 28, 2017, 6:08 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ktp-common-internals
</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;">Integrate Status Handler. API additions and cleanup. Bugfixes and documentation.</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;">Compile, run.</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>KTp/Declarative/qml-plugins.cpp <span style="color: grey">(27ed202be547c088feaba92664b64617e3e541fc)</span></li>
<li>KTp/Models/accounts-list-model.h <span style="color: grey">(09b1c28d1519dcfad24540008eab4ff0fadfd425)</span></li>
<li>KTp/Models/accounts-list-model.cpp <span style="color: grey">(216102658db0d9183ccfe851807db3117e2df1f9)</span></li>
<li>KTp/Models/presence-model.h <span style="color: grey">(7d35c9f529e51d1300926bc90a2674234ab87cfe)</span></li>
<li>KTp/Models/presence-model.cpp <span style="color: grey">(9d32174df8b1052f7f01a7e31c8f5cbf44a445ee)</span></li>
<li>KTp/global-presence.h <span style="color: grey">(46e6cc358b0f3cdaff35d3a7be25949646b400d3)</span></li>
<li>KTp/global-presence.cpp <span style="color: grey">(0b751b145a227d59de564baa21bed1df0b23b125)</span></li>
<li>KTp/presence.h <span style="color: grey">(901bd9898ae066e469cc6346d8f7433fef4108b2)</span></li>
<li>KTp/presence.cpp <span style="color: grey">(5ac92b1aba7d3b8fb86840b5e86c50b778b5c8dc)</span></li>
<li>KTp/types.h <span style="color: grey">(8d3ca4fc14a37b61abecdf78594f334f7b6aa797)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/130188/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>