<html>
<body>
<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="http://git.reviewboard.kde.org/r/105166/">http://git.reviewboard.kde.org/r/105166/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 21st, 2012, 2:08 p.m., <b>Matěj Laitl</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/105166/diff/2/?file=70025#file70025line34" style="color: black; font-weight: bold; text-decoration: underline;">tests/core/collections/TestCollection.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">class CollectionBaseMock : public CollectionBase</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">34</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="k">static</span> <span class="n">Capabilities</span><span class="o">::</span><span class="n">ActionsCapability</span> <span class="o">*</span><span class="n">s_actionsCapability</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Strange indentation again. Hmm?</pre>
</blockquote>
<p>On June 21st, 2012, 2:38 p.m., <b>Jasneet Bhatti</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;">Don't understand this one.
There are two levels of indentation and the variable definition is according to intro_and_style.
Please explain what is the fault.</pre>
</blockquote>
<p>On June 21st, 2012, 3:19 p.m., <b>Matěj Laitl</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;">There is no indentation from what I see here.</pre>
</blockquote>
<p>On June 21st, 2012, 3:24 p.m., <b>Jasneet Bhatti</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;">Well, the patch I uploaded and my repo both show the indentation, not sure why it isn't visible to you ?</pre>
</blockquote>
<p>On June 21st, 2012, 4:50 p.m., <b>Sam Lade</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;">There is indentation present, but again it's a tab rather than spaces (and tabs and spaces are mixed throughout the patch). Please use spaces only for indentation (see HACKING/intro_and_style.txt). You should configure your IDE or text editor to do this automatically to make life easier.</pre>
</blockquote>
<p>On June 21st, 2012, 5:05 p.m., <b>Jasneet Bhatti</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;">Oh I get it now. This will sound silly but I didn't know that four spaces and one tab of width four appear differently and so was confused about what mixing tabs and spaces meant. Will make sure to never press Tab again while writing patches. Thanks for clearing that.</pre>
</blockquote>
<p>On June 21st, 2012, 5:09 p.m., <b>Matěj Laitl</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;">> Will make sure to never press Tab again while writing patches.
No, you should absolutely press Tab otherwise your spacebar and thumb finger will go away. ;) Just configure your editor to add 4 spaces instead of tab or even better advance to next tab position by inserting spaces. (Kate (KDevelop) does this nicely)</pre>
</blockquote>
<p>On June 21st, 2012, 5:13 p.m., <b>Sam Lade</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;">That's not actually necessary - any reasonable text editor/IDE will have tab settings which allow you to configure it to use spaces for indentation, even when you press the tab key.
I don't know what you're using, but here's how I have some of the common ones set:
In Qt Creator, tools > options > text editor > behaviour > tabs and indentation (set to spaces only, tab size 8, indent size 4)
In Kate, settings > configure Kate > editing > indentation (spaces, tab width 4, indentation width 4, always advance to next tab position)
In vim, in .vimrc:
set softtabstop=4
set tabstop=8
set shiftwidth=4
set expandtab</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks, set my editor up with those.
BTW I can only imagine how annoying it must have been for you all to see the same mistake being repeated ;)
I apologize for the inconvenience</pre>
<br />
<p>- Jasneet</p>
<br />
<p>On June 21st, 2012, 1:38 p.m., Jasneet Bhatti wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Amarok and Sven Krohlas.</div>
<div>By Jasneet Bhatti.</div>
<p style="color: grey;"><i>Updated June 21, 2012, 1:38 p.m.</i></p>
<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 patch implements a unit test for core/collections/Collection
There are abstract classes to be tested as well, which can only be done when subclasses define the pure virtual functions. So tests for those will be done along with the subclasses.</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;">Test passes on my repository</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>tests/core/collections/CMakeLists.txt <span style="color: grey">(2efd1fe)</span></li>
<li>tests/core/collections/TestCollection.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/core/collections/TestCollection.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105166/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>