<p dir="ltr">Well this is embarrasing, my n00b-ishness is on display. I absolutely apologize for this and will make the required changes ASAP after going through the link. </p>
<p dir="ltr">You have been very helpful.</p>
<p dir="ltr">Thanks,<br>
Amarvir</p>
<div class="gmail_quote">On 4 Feb 2014 02:12, "Andreas Cord-Landwehr" <<a href="mailto:cordlandwehr@kde.org">cordlandwehr@kde.org</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
<div style="font-family:Verdana,Arial,Helvetica,Sans-Serif">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border:1px #c9c399 solid">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/115449/" target="_blank">https://git.reviewboard.kde.org/r/115449/</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">First of a short review of the coding style: Please take care to not use tabulators but 4-space indentations as well as proper spacing around operators, and variable naming.
Parley should mainly follow the KDElibs coding styles as explained here: <a href="http://techbase.kde.org/Policies/Kdelibs_Coding_Style" target="_blank">http://techbase.kde.org/Policies/Kdelibs_Coding_Style</a></pre>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line77" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right:1px solid #c0c0c0" align="right"><font>77</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>painter</span><span>-></span><span>drawPath</span><span>(</span><span>path</span><span>);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left:1px solid #c0c0c0;border-right:1px solid #c0c0c0" align="right"><font>62</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>float</span> <span>total</span> <span>=</span> <span>index</span><span>.</span><span>data</span><span>(</span><span>StatisticsModel</span><span>::</span><span>TotalCount</span><span>).</span><span>toInt</span><span>();</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">use "qreal"</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line78" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>63</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>int</span> <span>xsum</span> <span>=</span> <span>0</span><span>;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">rename to xPosition</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line82" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>67</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"> <span>int</span> <span>grade_width</span><span>=</span><span>(</span><span>double</span><span>)(</span><span>count</span><span>/</span><span>total</span><span>)</span><span>*</span><span>option</span><span>.</span><span>rect</span><span>.</span><span>width</span><span>();</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">* add spaces around operators
* use camel cased variables
* maybe something like barElementWidth would be mor appropriate as variable name?</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line83" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>68</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span><span> </span><span> </span><span> </span></span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">remove whitespace</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line84" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>69</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span> </span><span> </span><span> </span><span>QRectF</span> <span>grade_rect</span><span>(</span><span>option</span><span>.</span><span>rect</span><span>.</span><span>x</span><span>()</span><span>+</span><span>xsum</span><span>,</span><span>option</span><span>.</span><span>rect</span><span>.</span><span>y</span><span>(),</span><span>grade_width</span><span>,</span><span>option</span><span>.</span><span>rect</span><span>.</span><span>height</span><span>());</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">* use camel case variable names
* maybe barElement would better explain this variable?</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line85" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>70</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span> <span> </span></span><span>QPainterPath</span> <span>grade_path</span><span>;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">* camel case this variable
* maybe rename to elementBarPath?</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line89" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>74</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span> <span> </span></span><span>QPainterPath</span> <span>intersect_path</span> <span>=</span> <span>path</span><span>.</span><span>intersected</span><span>(</span><span>grade_path</span><span>);</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">* use camel case variable names
* maybe barElementBorder would be better?</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line91" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>76</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span> <span> </span></span><span>QColor</span> <span>color</span><span>=</span><span>Prefs</span><span>::</span><span>gradeColor</span><span>(</span><span>i</span><span>);</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">add spaces around operators</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line92" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>77</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span> <span> </span></span><span>color</span><span>.</span><span>setAlpha</span><span>(</span><span>255</span><span>-</span><span>(</span><span>7</span><span>-</span><span>i</span><span>)</span><span>*</span><span>35</span><span>);</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">this formula needs explenation (in particular, where are the values 7 and 35 coming from?)</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line93" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>78</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span> <span> </span></span><span>if</span><span>(</span><span>i</span><span>!=</span><span>0</span><span>){</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">add spaces: "if (i != 0) {"</pre>
</div>
<br>
<div>
<table width="100%" border="0" bgcolor="white">
<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/115449/diff/1/?file=241636#file241636line94" style="text-decoration:underline;font-weight:bold" target="_blank">src/statistics/lessonstatistics.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">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right:1px solid #c0c0c0" align="right"><font></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>79</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size:8pt;line-height:140%;margin:0"><span> </span><span> </span><span> </span><span> </span><span>painter</span><span>-></span><span>setBrush</span><span>(</span><span>QBrush</span><span>(</span><span>color</span><span>));</span></pre>
</td>
</tr>
</tbody>
</table>
<pre style="margin-left:2em;white-space:pre-wrap;white-space:-moz-pre-wrap;white-space:-pre-wrap;white-space:-o-pre-wrap;word-wrap:break-word">wrong indentation</pre>
</div>
<br>
<p>- Andreas Cord-Landwehr</p>
<br>
<p>On February 3rd, 2014, 6:06 p.m. UTC, Amarvir Singh wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image:url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png');background-repeat:repeat-x;border:1px black solid">
<tr>
<td>
<div>Review request for KDE Edu.</div>
<div>By Amarvir Singh.</div>
<p style="color:grey"><i>Updated Feb. 3, 2014, 6:06 p.m.</i></p>
<div style="margin-top:1.5em">
<b style="color:#575012;font-size:10pt">Repository: </b>
parley
</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">This closes the mentioned bug, as it removes the gradient in learning statistics and uses solid color.
It also makes it even more intuitive for the user, as it uses a single color for all the grades with different alpha values, increasing as grades increase. Thus darker shades mean higher grades.
Also the default grade colors have been changed to be the same, except grade 0.
Fixes BUG:#266368
</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">Tested the default button, and tested for all grades. Still intuitive and working.</pre>
</td>
</tr>
</table>
<h1 style="color:#575012;font-size:10pt;margin-top:1.5em">Diffs </h1>
<ul style="margin-left:3em;padding-left:0">
<li>src/settings/parley.kcfg <span style="color:grey">(34bfd98)</span></li>
<li>src/statistics/lessonstatistics.cpp <span style="color:grey">(e09cff8)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115449/diff/" style="margin-left:3em" target="_blank">View Diff</a></p>
</td>
</tr>
</table>
</div>
</div>
</blockquote></div>