<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/109384/">http://git.reviewboard.kde.org/r/109384/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 14th, 2013, 3:02 p.m. UTC, <b>Dan Vrátil</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;">The code looks fine, thanks!
There's another concern now, however (something that I haven't thought about before, sorry!) - if the code now depends on kdelibs, we don't fit Tear 1 in KDE Frameworks anymore. The question is whether we actually care, since this "workaround" can die with Solid2 asynchronous API, but still...</pre>
</blockquote>
<p>On March 15th, 2013, 6:19 a.m. UTC, <b>Alexander Mezin</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;">What do you mean by "can die"? Will break or won't be needed?</pre>
</blockquote>
<p>On March 15th, 2013, 9:17 a.m. UTC, <b>Alexander Mezin</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;">If you mean second. I think, regardless of the API, launching any application shouldn't cause drive spin-up, as it makes annoying noise and eats battery on laptop. However, if you don't read disks in a special daemon, you should read them in every application. And that causes frequent spin-ups. So there are three ways:
1) Use daemon
2) Don't detect video dvd's
3) Annoy users who have optical drives (the worst way. Please, don't do this again)</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;">Sadly, the code is not fine. It was thoroughly tested before converting to KDED module, but not after.
Other KDED modules want to call getContent. However, delayed reply doesn't work with loop-local messages. Also, sometimes KDED just hangs.
I think I can rewrite the module without any dependencies other than QtCore and QtDBus. This way it will fit into Tier 1, right?</pre>
<br />
<p>- Alexander</p>
<br />
<p>On March 14th, 2013, 3:31 a.m. UTC, Alexander Mezin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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 and Lukáš Tinkl.</div>
<div>By Alexander Mezin.</div>
<p style="color: grey;"><i>Updated March 14, 2013, 3:31 a.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;">While bug 261552 marked as fixed, optical drive spin-ups still happen more frequent than needed. Also, if there is a corrupted optical disk in the drive, a long delay happens when opening new file manager window (bug 306426).
This patch introduces a daemon that reads optical disks and remembers results, so disk access happens only once, usually right after new disk is inserted.
This is for udisks2 backend, but should also work with udisks 1. The daemon doesn't contain any backend-specific code.</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 tested it on top of kdelibs-4.10.1. Video DVDs are recognized correctly.</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="http://bugs.kde.org/show_bug.cgi?id=261552">261552</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=306426">306426</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>solid/solid/CMakeLists.txt <span style="color: grey">(ab3f554)</span></li>
<li>solid/solid/backends/udisks2/udisksopticaldisc.h <span style="color: grey">(0cdcc66)</span></li>
<li>solid/solid/backends/udisks2/udisksopticaldisc.cpp <span style="color: grey">(23a4fc1)</span></li>
<li>solid/solid/diskscan/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>solid/solid/diskscan/diskscanner.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>solid/solid/diskscan/diskscanner.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>solid/solid/diskscan/diskscannertask.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>solid/solid/diskscan/diskscannertask.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>solid/solid/diskscan/org.kde.Solid.DiskScanner.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>solid/solid/diskscan/soliddiskscan.desktop <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109384/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>