<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/104785/">http://git.reviewboard.kde.org/r/104785/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 1st, 2012, 9:55 a.m., <b>Konstantinos Smanis</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/104785/diff/3/?file=59754#file59754line734" style="color: black; font-weight: bold; text-decoration: underline;">kio/bookmarks/kbookmarkmenu.cc</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">KBookmarkAction::KBookmarkAction(const KBookmark &bk, KBookmarkOwner* owner, QObject *parent )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">733</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span><span class="k">this</span><span class="p">,</span> <span class="n">SIGNAL</span><span class="p">(</span> <span class="n">triggered</span><span class="p">(</span><span class="n">Qt</span><span class="o">::</span><span class="n">MouseButtons</span><span class="p">,</span> <span class="n">Qt</span><span class="o">::</span><span class="n">KeyboardModifiers</span><span class="p">)</span> <span class="p">),</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">733</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">connect</span><span class="p">(</span><span class="k">this</span><span class="p">,</span> <span class="n">SIGNAL</span><span class="p">(</span><span class="n">triggered</span><span class="p">(</span><span class="n">Qt</span><span class="o">::</span><span class="n">MouseButtons</span><span class="p">,</span> <span class="n">Qt</span><span class="o">::</span><span class="n">KeyboardModifiers</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;">You haven't normalized here.</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;">Well well well... The Qt tool only looked for ".cpp" files and ignores those whose extension is .cc or .c++ or .cxx. I fixed that and it caught these as well.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 1st, 2012, 9:55 a.m., <b>Konstantinos Smanis</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/104785/diff/3/?file=59756#file59756line396" style="color: black; font-weight: bold; text-decoration: underline;">kio/kfile/kfilemetadataprovider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 KFileMetaDataProvider::setItems(const KFileItemList& items)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">396</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q_PRIVATE_SLOT</span><span class="p">(</span><span class="n">d</span><span class="p">,</span> <span class="kt">void</span> <span class="n">slotDataChangeStarted</span><span class="p">())</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">396</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q_PRIVATE_SLOT</span><span class="p">(</span><span class="n">d</span><span class="p">,</span><span class="kt">void</span> <span class="n">slotDataChangeStarted</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;">This doesn't need normalization. And since kdelibs coding style suggests padding operators, the whitespace should remain.</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;">It is the script that did that. However, I will revert this because it is indeed not necessary.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 1st, 2012, 9:55 a.m., <b>Konstantinos Smanis</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/104785/diff/3/?file=59775#file59775line6" style="color: black; font-weight: bold; text-decoration: underline;">kioslave/http/kcookiejar/kcookiejar_include.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#include <QtDBus/QtDBus></span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">Does it compile without the include?</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;">Yes that compiles fine. Even if certain includes are needed the ones that should have been included are QList and QMetaType, but neither is needed since this header is used in a generated source file that contains the necessary includes already.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 1st, 2012, 9:55 a.m., <b>Konstantinos Smanis</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;">I have made a few comments, mostly about code inconsistencies. Since it's a matter of taste of the maintainer I didn't open issues, except when needed.</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;">Though I appreciate you going through the patch and finding all of those styling issues, I think you completely neglected my last response to David's inquiry of whether I did the normalization fixes by hand or not. All the normalization related changes were done by a tool. It makes no sense to do these changes otherwise. As such, the last thing I worry or care about is the padding changes the tool might introduce to a bunch of connect statements. I got no time for such trivialities. With the exception of the code I manually added and or modified I am not going to go through all these changes and fix padding issues. Sorry.</pre>
<br />
<p>- Dawit</p>
<br />
<p>On April 30th, 2012, 11:50 p.m., Dawit Alemayehu 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 kdelibs.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated April 30, 2012, 11:50 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;">The following patch fixes the following krazy2 warnings:
- Use const references in Q_FOREACH statements where appropriate.
- Normalize yet more signal/slot connections (missing from the first go round).
- Use brackets instead of double-quotes for the 'config*' header files.
- Fix the #ifdef statements in header files to reflect the header filename.
I did this a long time ago, but never pushed upstream. As part of my spring clean up I want to push this local changes upstream. Any objections ?</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>kio/bookmarks/kbookmarkdialog.cc <span style="color: grey">(713ceff)</span></li>
<li>kio/bookmarks/kbookmarkdombuilder.cc <span style="color: grey">(8e0be3c)</span></li>
<li>kio/bookmarks/kbookmarkimporter.cc <span style="color: grey">(08210f7)</span></li>
<li>kio/bookmarks/kbookmarkmanager.cc <span style="color: grey">(d8a9cb7)</span></li>
<li>kio/bookmarks/kbookmarkmenu.cc <span style="color: grey">(deb973b)</span></li>
<li>kio/bookmarks/konqbookmarkmenu.cc <span style="color: grey">(4fc6be0)</span></li>
<li>kio/kfile/kfilemetadataprovider.cpp <span style="color: grey">(8caa0c2)</span></li>
<li>kio/kfile/kfilemetadataprovider_p.h <span style="color: grey">(09d924a)</span></li>
<li>kio/kfile/kfilemetadatareaderprocess.cpp <span style="color: grey">(5103087)</span></li>
<li>kio/kfile/kimagefilepreview.cpp <span style="color: grey">(74ef8b7)</span></li>
<li>kio/kio/accessmanager.cpp <span style="color: grey">(e535c8a)</span></li>
<li>kio/kio/chmodjob.cpp <span style="color: grey">(85e0c2c)</span></li>
<li>kio/kio/job.h <span style="color: grey">(aeaffa2)</span></li>
<li>kio/kio/job.cpp <span style="color: grey">(5e18998)</span></li>
<li>kio/kio/jobuidelegate.cpp <span style="color: grey">(85679c2)</span></li>
<li>kio/kio/kdesktopfileactions.cpp <span style="color: grey">(edf2e9c)</span></li>
<li>kio/kio/kfileitemactions.h <span style="color: grey">(27ab4e3)</span></li>
<li>kio/kio/kfileitemactions.cpp <span style="color: grey">(c79a434)</span></li>
<li>kio/kio/kfilemetainfoitem.cpp <span style="color: grey">(1cab458)</span></li>
<li>kio/kio/ksambasharedata.cpp <span style="color: grey">(aebcb04)</span></li>
<li>kio/kio/kurifilter.h <span style="color: grey">(289b910)</span></li>
<li>kio/kio/kurifilter.cpp <span style="color: grey">(0144a2c)</span></li>
<li>kio/kio/renamedialog.cpp <span style="color: grey">(11e55a9)</span></li>
<li>kio/misc/kpac/proxyscout.cpp <span style="color: grey">(0068ce7)</span></li>
<li>kio/misc/kpac/script.cpp <span style="color: grey">(a595301)</span></li>
<li>kioslave/http/kcookiejar/kcookiejar_include.h <span style="color: grey">(4053a53)</span></li>
<li>kioslave/http/tests/httpauthenticationtest.h <span style="color: grey">(35b822a)</span></li>
<li>nepomuk/core/resourcedata.cpp <span style="color: grey">(0a5295b)</span></li>
<li>nepomuk/query/dateparser.cpp <span style="color: grey">(9bd2e1a)</span></li>
<li>nepomuk/rcgen/ontologyparser.cpp <span style="color: grey">(f9f8673)</span></li>
<li>nepomuk/rcgen/property.cpp <span style="color: grey">(51d9c07)</span></li>
<li>nepomuk/types/class.cpp <span style="color: grey">(0210537)</span></li>
<li>nepomuk/types/ontology.cpp <span style="color: grey">(124e88c)</span></li>
<li>nepomuk/types/ontologymanager.cpp <span style="color: grey">(10d1031)</span></li>
<li>nepomuk/types/property.cpp <span style="color: grey">(06daf3c)</span></li>
<li>nepomuk/utils/datefacet.cpp <span style="color: grey">(386aa00)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104785/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>