<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/112721/">http://git.reviewboard.kde.org/r/112721/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 13th, 2013, 10:46 p.m. UTC, <b>Eike Hein</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;">(Also, just FWIW, not that the code matters given the above, but: You're caching a global setting and not handling the case that it can be changed at runtime, so the patch would have needed work anyway, even if its underlying assumption had been correct -- which as mentioned it isn't, the 3 is just a coincidence.)</pre>
</blockquote>
<p>On September 13th, 2013, 10:57 p.m. UTC, <b>Albert Vaca Cintora</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 already discarded the patch but, about this point, I think that lots of kde applications that read global settings have the same 'problem'. Actually for some of these settings there is already a warning saying that the running apps should be restarted, and I think we can live with that.</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;">I'm afraid I don't agree with that. Other things being broken isn't a good reason to write more broken things.
In most cases, changes to global settings can be adapted to at runtime by listening to the appropriate signals from KGlobalSettings (or Qt in KF5, since we've now eliminated KGlobalSettings and upstreamed its notification signals), and that's true here as well: KGlobalSettings::settingsChanged() is emitted with SETTINGS_QT when the wheel lines preference is changed, and the new value is subsequently available via QApplication:wheelScrollLines() (i.e. opening the kdeglobals config file was also unnecessary).
If you find cases where this isn't done correctly, please fix them. If there's a reason they haven't been fixed, let's fix the reason. We shouldn't "live with that".</pre>
<br />
<p>- Eike</p>
<br />
<p>On September 13th, 2013, 10:51 p.m. UTC, Albert Vaca Cintora 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 KDE Multimedia.</div>
<div>By Albert Vaca Cintora.</div>
<p style="color: grey;"><i>Updated Sept. 13, 2013, 10:51 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;">I noticed that changing the volume by scrolling over the KMix icon changed it in very big steps. I also realized that killing KDED made the step smaller and 'nicer' so I realized it might be a bug. I found that the scroll event was being called as many times as set in the setting "wheelScrollLines" ("Mouse wheel scrolls by [X] lines" in system settings). It makes sense when scrolling on documents, but not here, so I'm scaling the steps by that value so it always change in steps the same (smaller) size.</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;">Manual testing.</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>gui/kmixdockwidget.h <span style="color: grey">(0109086)</span></li>
<li>gui/kmixdockwidget.cpp <span style="color: grey">(e84338e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/112721/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>