<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/122680/">https://git.reviewboard.kde.org/r/122680/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 23rd, 2015, 11:23 a.m. UTC, <b>Martin Klapetek</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;">From the docs "Currently, the values set here are shown by the "About" box (see KAboutDialog), used by the bug report dialog (see KBugReport), and by the help shown on command line (see KAboutData::setupCommandLine())."</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So daemon has no About box but might need it for the KBugReport. And it doesn't implement --help anyway.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I think this might be fine for a daemon however, but for sure should be replaced by the QApplication bits:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">app.setApplicationVersion(aboutData.version());
app.setApplicationName(aboutData.componentName());
app.setApplicationDisplayName(aboutData.displayName());
app.setOrganizationDomain(aboutData.organizationDomain());</p></pre>
</blockquote>
<p>On February 23rd, 2015, 11:38 a.m. UTC, <b>Martin Gräßlin</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 agree that the application data needs to be set, so that bug reports in case of e.g. crashes work.</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;">So would something like this be acceptable?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%">app.setApplicationVersion(<span style="color: #BA2121">"0.2"</span>);
app.setApplicationName(<span style="color: #BA2121">"kglobalaccel"</span>);
app.setApplicationDisplayName(tr(<span style="color: #BA2121">"KDE Global Shortcuts Service"</span>));
app.setOrganizationDomain(<span style="color: #BA2121">"kde.org"</span>);
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I guess the tr() would not be accepted, but bringing in k18n because of the <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">one</em> string which will almost never appear is a huge shame.</p></pre>
<br />
<p>- Jerome</p>
<br />
<p>On February 23rd, 2015, 10:34 a.m. UTC, Jerome Leclanche 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, Martin Gräßlin and Martin Klapetek.</div>
<div>By Jerome Leclanche.</div>
<p style="color: grey;"><i>Updated Feb. 23, 2015, 10:34 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kglobalaccel
</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;">Remove the runtime's KAboutData
The about data was unexposed, but created a dependency on KCoreAddons (for
KAboutData) and in turn on KI18n for the translations of the aboutData.
This removes both dependencies as well as the string extraction scripts.
--
Author notes: This is a RFC. We don't use kglobalaccel in LXQt but we would
like to, however it currently has too many dependencies. See
https://github.com/lxde/lxqt/issues/507 for related discussion.
I'm unsure myself if the about data is actually exposed somewhere I completely
missed, but it doesn't look that way.</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;">Compiles and runs. No further testing done.</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>CMakeLists.txt <span style="color: grey">(68ad795)</span></li>
<li>src/Messages.sh <span style="color: grey">(8eae937)</span></li>
<li>src/runtime/CMakeLists.txt <span style="color: grey">(e639fa5)</span></li>
<li>src/runtime/Messages.sh <span style="color: grey">(8a5e4a9)</span></li>
<li>src/runtime/globalshortcutsregistry.cpp <span style="color: grey">(3e4d720)</span></li>
<li>src/runtime/kglobalacceld.cpp <span style="color: grey">(4e7cb9d)</span></li>
<li>src/runtime/main.cpp <span style="color: grey">(fdf4d62)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122680/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>