<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 />
<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>
<br />
<div>
<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/122196/diff/1/?file=344087#file344087line166" style="color: black; font-weight: bold; text-decoration: underline;">applets/binary-clock/binaryclock.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void BinaryClock::createClockConfigurationInterface(KConfigDialog *parent)</pre></td>
</tr>
</tbody>
<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">166</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span><span class="n">ui</span><span class="p">.</span><span class="n">usePerFieldBinaryCheckBox</span><span class="p">,</span> <span class="n">SIGNAL</span><span class="p">(</span><span class="n">stateChanged</span><span class="p">(</span><span class="kt">int</span><span class="p">)),</span> <span class="n">parent</span><span class="p">,</span> <span class="n">SLOT</span><span class="p">(</span><span class="n">settingsModified</span><span class="p">()));</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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;">You probably want to read the value of perFieldBinary and initialize the checkbox with it. Otherwise it'll always default to false, no matter the actual setting.</p></pre>
</div>
</div>
<br />
<div>
<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/122196/diff/1/?file=344087#file344087line179" style="color: black; font-weight: bold; text-decoration: underline;">applets/binary-clock/binaryclock.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void BinaryClock::createClockConfigurationInterface(KConfigDialog *parent)</pre></td>
</tr>
</tbody>
<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">179</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">m_usePerFieldBinary</span> <span class="o">=</span> <span class="n">ui</span><span class="p">.</span><span class="n">usePerFieldBinaryCheckBox</span><span class="o">-></span><span class="n">isChecked</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<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 think you're forgetting to actually save the config value. Look at the cg.writeEntry(...) calls below.</p></pre>
</div>
</div>
<br />
<p>- Sebastian Kügler</p>
<br />
<p>On January 22nd, 2015, 6:47 a.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, 6:47 a.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">(3b8c7af4631fcb60437045eae37d0c0552c85299)</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>