<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/121534/">https://git.reviewboard.kde.org/r/121534/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 16th, 2014, 9:01 p.m. UTC, <b>Etienne Rebetez</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 dont know why KUnitConversion::UnitID was used. Probably just to show in the code, that this is an unit id?
Thanks for the porting initative:) I dont see the branch frameworks in git? Is there another repo?
Thanks.</p></pre>
</blockquote>
<p>On December 16th, 2014, 9:24 p.m. UTC, <b>Christoph Feck</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;">No, because KUnitConversion framework was ported from int to enum</p></pre>
</blockquote>
<p>On December 16th, 2014, 9:25 p.m. UTC, <b>Christoph Feck</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;">And there is no repo yet :)</p></pre>
</blockquote>
<p>On December 17th, 2014, 9:44 p.m. UTC, <b>Etienne Rebetez</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;">Uups, sorry. I would be good if I could read...
Yes, enum is the nicer way to go. IMHO I the verbose way does not bother me.</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;">Well, I did not like doing the changes for all Prefs::xxxUnit() accessors, see https://paste.kde.org/p04lci6lb</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I had hoped kconfig_compiler can create accessors which return an enum instead of an int, but it apparently cannot.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">My current idea is to keep the Kalzium code all using "int", but only convert this to the enum when actually doing the KUnitConversion calls, instead of porting all of Kalzium to using the enums.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Which would you prefer?</p></pre>
<br />
<p>- Christoph</p>
<br />
<p>On December 15th, 2014, 9:47 p.m. UTC, Christoph Feck 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 KDE Edu and Etienne Rebetez.</div>
<div>By Christoph Feck.</div>
<p style="color: grey;"><i>Updated Dec. 15, 2014, 9:47 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kalzium
</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;">Not sure if anyone already looked at porting Kalzium. If not, I would like to create the frameworks branch, and push some changes I tried so far.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">When I started porting the buildsystem I noticed many compile errors caused by API changes in KUnitConversion.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This review request only shows the changes inside libscience. The application does not compile yet.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I would like feedback about the (rather verbose) usage of KUnitConversion::UnitID type. Does it make sense to always "using namespace;" or maybe a typedef?</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;">libscience compiles without errors/warnings.</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>libscience/chemicaldataobject.h <span style="color: grey">(55a40dc)</span></li>
<li>libscience/chemicaldataobject.cpp <span style="color: grey">(8fac92c)</span></li>
<li>libscience/element.h <span style="color: grey">(43979d9)</span></li>
<li>libscience/element.cpp <span style="color: grey">(762462f)</span></li>
<li>libscience/elementparser.h <span style="color: grey">(f3a8130)</span></li>
<li>libscience/elementparser.cpp <span style="color: grey">(7c5ecaf)</span></li>
<li>libscience/isotopeparser.cpp <span style="color: grey">(0e646c1)</span></li>
<li>libscience/spectrum.h <span style="color: grey">(3d43d6f)</span></li>
<li>libscience/spectrum.cpp <span style="color: grey">(9ce97a5)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121534/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>