<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127635/">https://git.reviewboard.kde.org/r/127635/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 17th, 2016, 10:49 a.m. UTC, <b>Martin Tobias Holmedahl Sandsmark</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Just briefly looking around at what's using Sonnet, at least Calligra and Quassel uses it on Windows, so I'd really like to migrate the settings on Windows.</p></pre>
</blockquote>
<p>On April 17th, 2016, 10:54 a.m. UTC, <b>Martin Tobias Holmedahl Sandsmark</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;">it should ideally have a unit test as well, but considering that this would be something temporary it's not a dealbreaker.
(I also saw that Otter Browser uses it, which I guess is pretty popular on Windows.)</pre>
</blockquote>
<p>On April 17th, 2016, 11:35 a.m. UTC, <b>René J.V. Bertin</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As I said, I have nothing against adding a migration path, but I'd want some pointers on how to implement that. And also know if we really don't need to support both formats during a transition period.
To repeat what I said on the ML: with a packaging approach in which each installed application tends to come with its own copies of all required dependencies, one cannot assume that all those applications automagically get to use a Sonnet library that uses the new format and has an importer for the old format (ditto for the other frameworks that'll need to be patched).
That almost calls for an approach that detects presence of settings in the old format, imports those if they're newer than those in the new format store, and also keeps that old format store up to date when saving. IOW, apps using the new Sonnet library would need to pick up settings changes made by apps still using an older version, and vice versa.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If that's over the top then I'd say it's almost better not to do any migration at all, because that at least will send a clear message to users. "New version of app didn't pick up certain settings, so I best not expect that any changes I make in that app will be seen from other apps".</p></pre>
</blockquote>
<p>On April 17th, 2016, 1:35 p.m. UTC, <b>Martin Tobias Holmedahl Sandsmark</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I imagine something like this: http://ix.io/wVw</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">But it needs testing from someone who has Windows. I'm not sure if the .ini file is shared amongst all applications for a single user.</p></pre>
</blockquote>
<p>On April 17th, 2016, 3:57 p.m. UTC, <b>René J.V. Bertin</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yeah, that looks like it would work, but it should be done like that only if the settings are application-specific (I had not considered they might be).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't really like the idea to include such a change in my patch as I won't be able to test it (apart from checking if it builds when I remove the #ifdefs).</p></pre>
</blockquote>
<p>On November 20th, 2016, 8:59 p.m. UTC, <b>René J.V. Bertin</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What's to stop Sonnet from using a settings category per application, or else a subdirectory per application?</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Nothing, really, but there needs to be global defaults as well.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As for the bump, it still needs to migrate the settings on Windows, as well as testing on Windows.</p></pre>
<br />
<p>- Martin Tobias Holmedahl</p>
<br />
<p>On November 20th, 2016, 7:44 p.m. UTC, René J.V. Bertin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.</div>
<div>By René J.V. Bertin.</div>
<p style="color: grey;"><i>Updated Nov. 20, 2016, 7:44 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
sonnet
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">As discussed on kde-frameworks-devel, this is a proposal to use QSettingsIniFormat in order to have the same file format across platforms and avoid storing settings in the registry under MS Windows.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The ML discussion has touched on the idea of backward compatibility (at least migrating settings from the NativeFormat where this is different from IniFormat). The current patch doesn't yet include anything like that.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">On OS X 10.9.5 and Linux (KUbuntu 14.04) with Qt 5.6.0 and FWs 5.20.0 installed into /opt/local .</p></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>autotests/test_settings.cpp <span style="color: grey">(f2bc80c)</span></li>
<li>src/core/settings.cpp <span style="color: grey">(8614e35)</span></li>
<li>src/plugins/hspell/hspelldict.cpp <span style="color: grey">(b9265eb)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127635/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>