<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="https://git.reviewboard.kde.org/r/117806/">https://git.reviewboard.kde.org/r/117806/</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;">Also, have you run the ModelTest against it?</pre>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/117806/diff/2/?file=268753#file268753line78" style="color: black; font-weight: bold; text-decoration: underline;">src/imports/declarativedevices.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">78</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q_PROPERTY</span><span class="p">(</span><span class="kt">bool</span> <span class="n">empty</span> <span class="n">READ</span> <span class="n">isEmpty</span> <span class="n">NOTIFY</span> <span class="n">emptyChanged</span><span class="p">)</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;">I would remove this property. It adds quite some logic and complexity for no apparent reason. You can use count==0 instead.</pre>
</div>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/117806/diff/2/?file=268754#file268754line166" style="color: black; font-weight: bold; text-decoration: underline;">src/imports/declarativedevices.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">166</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">m_backend</span><span class="p">)</span> <span class="p">{</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;">You probably want to do a begin/endRemoveRows or begin/endResetModel.</pre>
</div>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/117806/diff/2/?file=268754#file268754line189" style="color: black; font-weight: bold; text-decoration: underline;">src/imports/declarativedevices.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">189</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">beginInsertRows</span><span class="p">(</span><span class="n">QModelIndex</span><span class="p">(),</span> <span class="n">rowCount</span><span class="p">()</span> <span class="o">-</span> <span class="mi">1</span><span class="p">,</span> <span class="n">rowCount</span><span class="p">()</span> <span class="o">-</span> <span class="mi">1</span><span class="p">);</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;">You want to actually add it here, no?
</pre>
</div>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/117806/diff/2/?file=268754#file268754line215" style="color: black; font-weight: bold; text-decoration: underline;">src/imports/declarativedevices.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">215</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">endRemoveRows</span><span class="p">();</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;">No need to remove anything?
</pre>
</div>
<br />
<div>
<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="https://git.reviewboard.kde.org/r/117806/diff/2/?file=268760#file268760line118" style="color: black; font-weight: bold; text-decoration: underline;">src/solid/CMakeLists.txt</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">117</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="s">backends/udev/udevdeviceinterface.cpp</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">118</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c"><span class="hl">#</span>backends/udev/udevdeviceinterface.cpp</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;">Don't comment, remove, no?</pre>
</div>
<br />
<p>- Aleix Pol Gonzalez</p>
<br />
<p>On April 27th, 2014, 9:41 p.m. UTC, Kai Uwe Broulik 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-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Solid, Àlex Fiestas, Aleix Pol Gonzalez, Ivan Čukić, and Lukáš Tinkl.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated April 27, 2014, 9:41 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
solid
</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 adds proper model support to the Solid device import making it possible to hook it up to a ListView directly with automatically propagated change notifications.
You can have a look at a WIP battery monitor using it in [plasma-workspace] broulik/batterymonitorsolidimport branch.
I'm not completely satisfied with it as I'm not really good at that Qt model stuff, having the private impl mess around with the QList and having the model itself just calling beginInsertRows and endInsertRows is probably not optimal.
I apologize for the hard-to-read diff but I renamed the files outside of git and git diff --find-copies-harder confused Review Board :/ It is also available in the broulik/modelimport branch.</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;">I tried to iron out most of the crashes and issues and that thing will definitely need a whole lot of unit tests.</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>src/imports/CMakeLists.txt <span style="color: grey">(8c579ef)</span></li>
<li>src/imports/declarativedevices.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/imports/declarativedevices.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/imports/declarativedevices_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/imports/devices.h <span style="color: grey">(e321c5c)</span></li>
<li>src/imports/devices.cpp <span style="color: grey">(a586728)</span></li>
<li>src/imports/devices_p.h <span style="color: grey">(7d5a1f9)</span></li>
<li>src/imports/solidextensionplugin.cpp <span style="color: grey">(6a959f1)</span></li>
<li>src/solid/CMakeLists.txt <span style="color: grey">(8fb91da)</span></li>
<li>src/solid/backends/fakehw/fakebattery.h <span style="color: grey">(f544613)</span></li>
<li>src/solid/backends/fakehw/fakebattery.cpp <span style="color: grey">(9af4c6c)</span></li>
<li>src/solid/backends/fakehw/fakedeviceinterface.h <span style="color: grey">(c704b44)</span></li>
<li>src/solid/backends/fakehw/fakedeviceinterface.cpp <span style="color: grey">(b5f5821)</span></li>
<li>src/solid/backends/fstab/fstabnetworkshare.h <span style="color: grey">(775999c)</span></li>
<li>src/solid/backends/fstab/fstabnetworkshare.cpp <span style="color: grey">(86f56e6)</span></li>
<li>src/solid/backends/fstab/fstabstorageaccess.h <span style="color: grey">(d608387)</span></li>
<li>src/solid/backends/fstab/fstabstorageaccess.cpp <span style="color: grey">(ca18502)</span></li>
<li>src/solid/backends/hal/halbattery.h <span style="color: grey">(71d654d)</span></li>
<li>src/solid/backends/hal/halbattery.cpp <span style="color: grey">(a2ac83d)</span></li>
<li>src/solid/backends/hal/halblock.h <span style="color: grey">(f0310f1)</span></li>
<li>src/solid/backends/hal/halblock.cpp <span style="color: grey">(1d3e51c)</span></li>
<li>src/solid/backends/hal/halcamera.h <span style="color: grey">(c7c339e)</span></li>
<li>src/solid/backends/hal/halcamera.cpp <span style="color: grey">(562568d)</span></li>
<li>src/solid/backends/hal/halcdrom.cpp <span style="color: grey">(b35b0de)</span></li>
<li>src/solid/backends/hal/haldevice.cpp <span style="color: grey">(e878576)</span></li>
<li>src/solid/backends/hal/halgenericinterface.h <span style="color: grey">(400bd10)</span></li>
<li>src/solid/backends/hal/halgenericinterface.cpp <span style="color: grey">(db7cd40)</span></li>
<li>src/solid/backends/hal/halopticaldisc.cpp <span style="color: grey">(b3004be)</span></li>
<li>src/solid/backends/hal/halportablemediaplayer.h <span style="color: grey">(e82e825)</span></li>
<li>src/solid/backends/hal/halportablemediaplayer.cpp <span style="color: grey">(fde65bd)</span></li>
<li>src/solid/backends/hal/halprocessor.h <span style="color: grey">(4f75136)</span></li>
<li>src/solid/backends/hal/halprocessor.cpp <span style="color: grey">(31959ae)</span></li>
<li>src/solid/backends/hal/halstorage.cpp <span style="color: grey">(7ded650)</span></li>
<li>src/solid/backends/hal/halstorageaccess.h <span style="color: grey">(f8ad7d1)</span></li>
<li>src/solid/backends/hal/halstorageaccess.cpp <span style="color: grey">(7463212)</span></li>
<li>src/solid/backends/hal/halvolume.cpp <span style="color: grey">(4dd4bcc)</span></li>
<li>src/solid/backends/udev/udevblock.h <span style="color: grey">(294b157)</span></li>
<li>src/solid/backends/udev/udevblock.cpp <span style="color: grey">(d7dc110)</span></li>
<li>src/solid/backends/udev/udevcamera.h <span style="color: grey">(3b0b1a4)</span></li>
<li>src/solid/backends/udev/udevcamera.cpp <span style="color: grey">(fa89055)</span></li>
<li>src/solid/backends/udev/udevdeviceinterface.h <span style="color: grey">(5820a6d)</span></li>
<li>src/solid/backends/udev/udevdeviceinterface.cpp <span style="color: grey">(b67592c)</span></li>
<li>src/solid/backends/udev/udevgenericinterface.h <span style="color: grey">(b7a0b2b)</span></li>
<li>src/solid/backends/udev/udevgenericinterface.cpp <span style="color: grey">(f513cec)</span></li>
<li>src/solid/backends/udev/udevportablemediaplayer.h <span style="color: grey">(6cb416c)</span></li>
<li>src/solid/backends/udev/udevportablemediaplayer.cpp <span style="color: grey">(907051c)</span></li>
<li>src/solid/backends/udev/udevprocessor.h <span style="color: grey">(f424aab)</span></li>
<li>src/solid/backends/udev/udevprocessor.cpp <span style="color: grey">(80d14f5)</span></li>
<li>src/solid/backends/udisks2/udisksdeviceinterface.h <span style="color: grey">(871aad9)</span></li>
<li>src/solid/backends/udisks2/udisksdeviceinterface.cpp <span style="color: grey">(9fa60e5)</span></li>
<li>src/solid/backends/upower/upowerbattery.h <span style="color: grey">(f0f41b5)</span></li>
<li>src/solid/backends/upower/upowerbattery.cpp <span style="color: grey">(c771f68)</span></li>
<li>src/solid/backends/upower/upowerdeviceinterface.h <span style="color: grey">(2fcb322)</span></li>
<li>src/solid/backends/upower/upowerdeviceinterface.cpp <span style="color: grey">(0ee5674)</span></li>
<li>src/solid/battery.h <span style="color: grey">(bbf2201)</span></li>
<li>src/solid/battery.cpp <span style="color: grey">(3cc376f)</span></li>
<li>src/solid/device.cpp <span style="color: grey">(93d77e0)</span></li>
<li>src/solid/deviceinterface.h <span style="color: grey">(dd212cc)</span></li>
<li>src/solid/deviceinterface.cpp <span style="color: grey">(7c7222f)</span></li>
<li>src/solid/ifaces/backenddeviceinterface.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/solid/ifaces/backenddeviceinterface.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/solid/ifaces/battery.h <span style="color: grey">(864f93d)</span></li>
<li>src/solid/ifaces/device.h <span style="color: grey">(893823d)</span></li>
<li>src/solid/ifaces/deviceinterface.h <span style="color: grey">(5c14496)</span></li>
<li>src/solid/ifaces/deviceinterface.cpp <span style="color: grey">(32ec160)</span></li>
<li>src/solid/managerbase.cpp <span style="color: grey">(8eb97ad)</span></li>
<li>src/solid/udevdeviceinterface.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/solid/udevdeviceinterface.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117806/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>