<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/123485/">https://git.reviewboard.kde.org/r/123485/</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 22nd, 2017, 8:14 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;">As I have explained many many times, I will *not* accept any patch including/extending plugin queue. The whole plugin queue has to go together with the mpris2 thing.
I'm sorry but if you don't intend to work towards that and/or work as a team towards common goal of this project, I will have to ask you to just discard this altogether.</pre>
</blockquote>
<p>On February 27th, 2017, 5:35 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;">I'd prefer to keep it open for now. If someone <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">really</em> has an issue with it still being open, then perhaps we can get some additional comments or 'ship it!'s.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My current understanding is that you don't like the status message capability, and autoaway capability.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The presence queue logic has to stay, to replace existing (broken) functionality in GlobalPresence. Since the status message counterpart of this functionality is also useful for setting a global status message independent of per-account presences, there are no savings to eliminating the mpris plugin, just loss of (already existing) functionality. The presence infrastructure is a) already shared with status message plugins, and b) required for juggling multiple presence possibilities, and c) a plugin is a logical way to replace per-app save / restore presence. There are no savings this way in removing AutoAway functionality either.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm not completely against removing the AutoAway plugin, and I'd personally like to keep the mpris plugin, and the plugin queue itself is useful.</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;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My current understanding is that you don't like the status message capability, and autoaway capability.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Your understanding is wrong. I want simple autoaway and lock screen integration, that's all. There is no queue needed for this nor any incredible complexity as everything would become mutually exclusive - you either have autoaway xor lock screen status xor your own status. It's that simple. If GlobalPresence is broken, then it needs fixing but not by introducing thousands new lines of code. If it cannot be fixed, it should be rewritten from scratch while being as simple as possible.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This whole patch group is a no-go and as the acting project maintainer I will not give it any approval now or next year.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If you really want this in, I suggest you fork the KDE Telepathy project and start shipping your fork.</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On February 22nd, 2017, 7:52 p.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, Dominik Cermak, David Edmundson, Lucas Betschart, and Martin Klapetek.</div>
<div>By James Smith.</div>
<p style="color: grey;"><i>Updated Feb. 22, 2017, 7:52 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=293443">293443</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=297924">297924</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=334542">334542</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=337881">337881</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=340201">340201</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=340781">340781</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ktp-kded-module
</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:
-AccountStatusHelper, a multiple-account-presence QDBusAbstractInterface replacement for AutoConnect.
-PluginQueue, a QDBusAbstractInterface for presence and status message plugins.
-Presence / Status Message Inserter classes, for queuing presences and status messages.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New properties:
-TelepathyKDEDModulePlugin: KTp::PluginQueue pluginState().</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New methods:
-TelepathyKDEDModulePlugin: virtual pluginInit(bool), method setPlugin(State), method setPlugin(KTp::Presence), method setPlugin(QString), method configureEnabled(bool).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">New features:
-Newly enabled and online account presence changes to account requested presence, or plugin presence depending on autoconnect property, and plugin queue state.
-Offline global presence will override per-account presences temporarily.
-PluginQueue globs presence change events.
-StatusHandler dynamically disables and enables plugin queues if there are online accounts.
-StatusHandler and TelepathyKDEDModulePlugin no longer require GlobalPresence.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Various other cleanups, documentation, and improvements.</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>CMakeLists.txt <span style="color: grey">(4dbaab64189d819a27e5b36fd10098f0fac3dbc6)</span></li>
<li>account-status-helper.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>account-status-helper.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>autoaway.h <span style="color: grey">(5e2a56100b56e6d21bc649f9032c8bece9031452)</span></li>
<li>autoaway.cpp <span style="color: grey">(4881c71afab2be2468c3c883523d2844865e6247)</span></li>
<li>autoconnect.h <span style="color: grey">(0f3ab137cf67cce9c7467144a45d1706ec9cc007)</span></li>
<li>autoconnect.cpp <span style="color: grey">(99512b265e0e6ae34f83263ffa6337741db652ea)</span></li>
<li>config/telepathy-kded-config.cpp <span style="color: grey">(88220645c2e119dc7879cdae065cbbf06aa13329)</span></li>
<li>config/telepathy-kded-config.ui <span style="color: grey">(2b80dfa34381af2e9206384a31c025f9b914854a)</span></li>
<li>org.kde.Telepathy.AccountStatusHelper.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>org.kde.Telepathy.PluginQueue.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>screensaveraway.h <span style="color: grey">(92344e891ccde16d53771c3e279d845f4d800b2d)</span></li>
<li>screensaveraway.cpp <span style="color: grey">(f226564a5bf7b396965b5ae21f81d93b7edc3ef8)</span></li>
<li>status-handler-plugin-queue.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>status-handler-plugin-queue.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>status-handler.h <span style="color: grey">(06240ff17e22148f2b128bc0eb8cec6d6abe68ff)</span></li>
<li>status-handler.cpp <span style="color: grey">(4b9c25a2ccba451f6e608bb704626e33149108cc)</span></li>
<li>telepathy-kded-module-plugin.h <span style="color: grey">(4c161696a706e82059a7eb314773c3644fe26bd7)</span></li>
<li>telepathy-kded-module-plugin.cpp <span style="color: grey">(daf73c66947bc946097de7a8e8a1518555131145)</span></li>
<li>telepathy-module.h <span style="color: grey">(17ef4cef27b90cbced6c66846ed82cb5c36fb532)</span></li>
<li>telepathy-module.cpp <span style="color: grey">(b74053bc1817d773a4fe37769e42f4013ced5425)</span></li>
<li>telepathy-mpris.h <span style="color: grey">(05b77c90a50372fd9ed66bde0ab8a287caf34b51)</span></li>
<li>telepathy-mpris.cpp <span style="color: grey">(ee0e622c68bdd156e45914f542d2fe13f0ddb610)</span></li>
<li>telepathy-status-inserter.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>telepathy-status-inserter.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/CMakeLists.txt <span style="color: grey">(7ec77495417a6790060ea5ea7126c46399dff755)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123485/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>