<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/123482/">https://git.reviewboard.kde.org/r/123482/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 29th, 2016, 8:45 p.m. CET, <b>Martin Klapetek</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'm not really sure I want to have this incredibly complex feature-set for the little gain it provides.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't know what your overall goal with this set of changes actually is as there is no explanation
anywhere, just a bunch of patches. But I assume it has to do with the kded plugins and per-account
presences. I would actually like to just kill the kded plugins and leave just auto-away. And the plugins
presence handling actually becomes very simple - it's either set by a plugin or not. I don't see a need
for a whole dbus infrastructure to mess with presences.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for your StatusHandlerController, that seems kinda useless. You either controll all accounts by
using the GlobalPresence, or you're using separate account presences, at which point the GlobalPresence
should simply have two states. You don't need no special class nor dbus interfaces to control per-account
presences, you can do that directly.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So, to summarize:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">GlobalPresence should have a "global" and "non-global" state</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">All our presence controls should have the same two modes</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Setting the non-global mode from the UI will disable setting all accounts to the same presence</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Setting global presence from the UI will again set everything</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Auto-away overrides all presences</li>
</ul></pre>
</blockquote>
<p>On March 7th, 2016, 8:08 a.m. CET, <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 KDE global / account presence setting should as far as possible be done compatibly with how TelepathyQt can accept incoming presences from other IM applications. This increases complexity a bit because we have to take into account our own automatic presences, and on top of this we do our own auto connect. This patchset throws in control and monitor functions that cover what sticking to the basics doesn't actually allow for, in addition to changing some existing shortcomings, and addressing other noticeable flaws.. Having a non-global mode to set is probably not a step in the right direction.</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;">> Having a non-global mode to set is probably not a step in the right direction.
So is not adding 3000 tons of complexity for 3 (effectively 2)
presence plugins that the kded has.
I wanted the mpris plugin long gone, it's just not useful anymore,
which leaves auto-away and screensaver-away that can be merged into
one.
I'm all for fixing our current code by simplifying things, but I'm sorry
I will not accept *this* much code for handling what effectively should
be a single presence plugin that overrides everything.
I'd rather we spent our time on making things simpler and focusing on what
actually matters - like Telegram support - and remove old stuff that should
just die already.</pre>
<br />
<p>- Martin</p>
<br />
<p>On March 7th, 2016, 7:53 a.m. CET, 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 March 7, 2016, 7:53 a.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;">New classes:
-StatusHandlerPresence, an interface for StatusHandler PluginQueue and AccountStatusHelper DBus interfaces, and a number of control functions for the Presence / StatusMessageInserter plugins (per-app save / restore presence replacement).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New properties:
-GlobalPresence: bool activeAutoPresence(), bool mixedPresenceMode(), KTp::Presence globalRequestedPresence().</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New methods:
-GlobalPresence: onAccountDisabled(Tp::AccountPtr)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New types:
KTp::PluginQueue, for indicating the state of the plugin queue plugins, e.g. PluginDisabledOnLoad, PluginDisabled, PluginEnabled, PluginActive, etc.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New features:
-pluginQueue active / inactive information in GlobalPresence, separate presenceQueueActiveChanged and statusMessageQueueActiveChanged signals.
-More reliable account property signalling.
-Reworked account enable / disable.
-Presence priority improvements.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Other simple cleanups (Use KTp::Presence where possible, make function arguments conform, commenting clarifications, errant properties, etc.)</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/CMakeLists.txt <span style="color: grey">(85d578bc3cbc0ae5769e6ff9f466381da4e701a7)</span></li>
<li>KTp/Declarative/qml-plugins.cpp <span style="color: grey">(4956a1c94fe60a526367f6a74a357d510e74e519)</span></li>
<li>KTp/Models/presence-model.cpp <span style="color: grey">(ddc1a7c75f1a452bf3ac2db1aecbd88a5d1ce519)</span></li>
<li>KTp/global-presence.h <span style="color: grey">(46e6cc358b0f3cdaff35d3a7be25949646b400d3)</span></li>
<li>KTp/global-presence.cpp <span style="color: grey">(8f1148b4ec339ab77f9c2fca0c87429ac14c5194)</span></li>
<li>KTp/presence.h <span style="color: grey">(7da07d0120c75ed057f4b2f2f832f5a1f80bd528)</span></li>
<li>KTp/presence.cpp <span style="color: grey">(6030db0ed821a4399603b5453ab124c741912a1f)</span></li>
<li>KTp/status-handler-presence.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>KTp/status-handler-presence.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>KTp/types.h <span style="color: grey">(27eb8975a4149e78c1e545b7273572c7eb18bad3)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123482/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>