<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/122196/">https://git.reviewboard.kde.org/r/122196/</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, 2015, 11:07 a.m. UTC, <b>Sebastian Kügler</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;">Code-wise, it has a few issues with the config saving and restoration, those are easily fixed, however.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Otherwise, the problem is that we've frozen Plasma 4.x for more than a year already, so we won't accept new features. All new feature development will have to be done against Plasma 5.x (in kdeplasma-addons, that would be the "frameworks" branch). I don't know about the porting status of the binary clock, but it'll need some adjustments in order to make it work, porting the UI to QML, for example. Best would be to port your patch to the Plasma 5.x version, and given the resulting code's quality is OK, I'd be quite fine with merging that.</p></pre>
</blockquote>
<p>On January 22nd, 2015, 12:53 p.m. UTC, <b>Patrick Uiterwijk</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;">Thanks for the feedback.
I checked the other branches, but there's no "frameworks" branch, just a "frameworks-scratch" branch that hasn't been touched since 2013.
There is however a Plasma/5.2 branch, which sounds like the right place, but it seems binary clock has not yet been ported to QML, and as such my patch directly applies there.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would be willing to learn QML and port binary-clock to it, though I would wonder if it would be okay to merge this patch before so I can port it as part of the rest?</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;">When porting, you'll find out that almost all portions of your patch will be removed during the port and rewritten in QML. It's not much use to merge your patch into a dead piece of code in a branch, it won't build, and you'll have to remove these bits again.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The only portion in your patch that also applies to the Plasma 5 version is the bit that modifies the XML definition of the config options, and as far as I can see, that bit is missing in your original patch.</p></pre>
<br />
<p>- Sebastian</p>
<br />
<p>On January 22nd, 2015, 12:54 p.m. UTC, Patrick Uiterwijk 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 Patrick Uiterwijk.</div>
<p style="color: grey;"><i>Updated Jan. 22, 2015, 12:54 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdeplasma-addons
</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;">Offer the option to show per-field binary instead of per-digit</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;">I have compiled a patched version of kdeplasma-addons on a recent version of Fedora (22), and am using it myself.</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>applets/binary-clock/binaryclock.h <span style="color: grey">(5ae83dfff9c473bd484b99d49330093b6e091975)</span></li>
<li>applets/binary-clock/binaryclock.cpp <span style="color: grey">(75c4548588ff15e2316dd1c2e888ad8c07f7f78b)</span></li>
<li>applets/binary-clock/clockConfig.ui <span style="color: grey">(0c2cec7925dd9e1ff92d7cd9d99005dca20497b0)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122196/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>