<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://svn.reviewboard.kde.org/r/5797/">http://svn.reviewboard.kde.org/r/5797/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 8th, 2010, 10:36 p.m., <b>Kevin Ottens</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;">Only minor whitespace issues. Also could you remove the foreach->Q_FOREACH changes, I don't see the point of it and we're using foreach everywhere else in the lib.</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;">It is always better to be safe than sorry and use a namespace construct as much as possible. Q_FOREACH is more likely to be namespace safe than the plain foreach definition. But it does not matter to me in this instance ; so I will revert it. Personally though I never use 'foreach' in code I write and encourge others not to do the same...</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 8th, 2010, 10:36 p.m., <b>Kevin Ottens</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://svn.reviewboard.kde.org/r/5797/diff/1/?file=40782#file40782line58" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdelibs/solid/solid/backends/udisks/udisksmanager.h</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 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">58</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">const</span> <span class="n">QStringList</span><span class="o">&</span> <span class="n">deviceCache</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;">s/QStringList& /QStringList &/
(I know the backend is kind of not homogeneous there, but that's the style used in libsolid).</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;">Funny how everyone has their own preference on that issue. Qt has it both ways, but the QtWebKit guys and webkit's style checker will reject code unless you do the way I normally do it, QStringList&. Anyhow, it is simple enough to fix the white space issues you pointed out...</pre>
<br />
<p>- Dawit</p>
<br />
<p>On November 8th, 2010, 7:39 a.m., Dawit Alemayehu wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 Solid.</div>
<div>By Dawit Alemayehu.</div>
<p style="color: grey;"><i>Updated 2010-11-08 07:39:06</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 attached patch does the following:
1.) Remove the call to allDevices() from UDisksManager's ctor. Instead a new
private method is added to properly initialize the devices list cache before
use.
2.) Remove the 'result' variable from UDisksManager::allDevices since it is
unnecessary and m_deviceCache can be used directly.
3.) Other minor namespace (foreach -> Q_FOREACH) and macro-performance (moving
the call for propertyCount() out of the for loop.
Note that the Bug number above is only mentioned as a reference to point out the origins of the attached patches...</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=253039">253039</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>trunk/KDE/kdelibs/solid/solid/backends/udisks/udisksdevice.cpp <span style="color: grey">(1192709)</span></li>
<li>trunk/KDE/kdelibs/solid/solid/backends/udisks/udisksmanager.h <span style="color: grey">(1192709)</span></li>
<li>trunk/KDE/kdelibs/solid/solid/backends/udisks/udisksmanager.cpp <span style="color: grey">(1192709)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5797/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>