<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/117275/">https://git.reviewboard.kde.org/r/117275/</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 12th, 2014, 9:16 a.m. UTC, <b>David Faure</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;">Ship It!</pre>
</blockquote>
<p>On April 12th, 2014, 9:19 a.m. UTC, <b>David Faure</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;">Well, I guess the first diff minimizes the porting effort indeed.
Also: the domain name can only be passed to QCoreApp if this is the main aboutdata (we also have a KAboutData per plugin).
But yeah, that seems to be missing in KAboutData::setApplicationData.</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 could also combine them and add both the short-form and long-form constructors. I guess the question is: do we want to push users towards a particular style? The "short constructors and setters" is what Qt has been moving towards, on the basis it makes more readable code; do we want to do the same with this class?
The organization domain stuff has a fallback to "kde.org"; currently this doesn't really matter, as we don't actually do anything with it. But if we're passing it to QCoreApplication, we should think about in the frameworks world (with a hopefully wider audience for these frameworks) we really want that default there.
It's easy enough to make the homepage setter change the organization domain if the org dom was never explicitly set (a bool flag would do the trick).</pre>
<br />
<p>- Alex</p>
<br />
<p>On April 1st, 2014, 10:09 a.m. UTC, Alex Merry 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 KDE Frameworks, David Faure, Kevin Ottens, and Michael Pyne.</div>
<div>By Alex Merry.</div>
<p style="color: grey;"><i>Updated April 1, 2014, 10:09 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kcoreaddons
</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 is another thing on the "if only we'd spotted it before beta1" list. I went with not allowing any optional arguments to the constructor, to encourage users of the class to use setters, which makes for more readable code. I deliberated giving it just one argument, but in the end went with the formerly-required arguments.
The organizationDomain is not automatically set from the home page with this new usage style, as that only happened in the constructor and not in the setter. It could be set if the organizationDomain has not been explicitly set. However, the organizationDomain is not passed to QCoreApplication as I assumed it would be - it that intentional?
Deprecate the catalog name stuff from KAboutData
This is pretty useless - the translation catalog has to be set before
KAboutData is constructed in order to translate its arguments.</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;">Builds, tests pass.</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/lib/kaboutdata.h <span style="color: grey">(cff1e3f67e33657fdd265a82166ef2a04cbcc3d1)</span></li>
<li>src/lib/kaboutdata.cpp <span style="color: grey">(ce64a13aaa89bb4bc077f05e5f8e175d6a441ead)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117275/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>