<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/114182/">http://git.reviewboard.kde.org/r/114182/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 28th, 2013, 3:43 p.m. UTC, <b>Kevin Ottens</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;">Looks fine to me. Makes me wonder if we want to drop the K for the target name "KF5::ConfigCore"? Just like Qt5Widgets becomes Qt5::Widgets...</pre>
</blockquote>
<p>On November 28th, 2013, 4:03 p.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;">Can do. Imho it makes sense but I'll wait for some other opinions so I don't have to redo/undo all of it then. Will try to still do it by tonight.</pre>
</blockquote>
<p>On November 28th, 2013, 7:46 p.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;">No comments on dropping the K? I'm about to do this...</pre>
</blockquote>
<p>On November 28th, 2013, 7:52 p.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;">+1</pre>
</blockquote>
<p>On November 28th, 2013, 9:32 p.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;">While working on it, I realized that it may not be so good to drop the K from target name after all...unless we change the package names too, otherwise we'd be finding package KConfig and linking to KF5::Config, plus all the classes are named KConfig*...so the library in target_link_libs would be quite inconsistent with the rest.
Please advise.</pre>
</blockquote>
<p>On November 28th, 2013, 9:54 p.m. UTC, <b>Alex Merry</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 we want to follow Qt's example, we would rename the package KF5Config, and leave the classes as KConfig*. This is a fairly substantial naming change to the packages, though, especially as the consistent thing would be to have KF5Solid and KF5ThreadWeaver, for example.
Which I'm generally in favour of, actually. I'm especially dubious about the names of the ItemModels and ItemViews packages, which are really quite generic.</pre>
</blockquote>
<p>On November 29th, 2013, 8:38 a.m. UTC, <b>Kevin Ottens</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;">Yep, ItemModels and ItemViews would definitely benefit from being prefixed. XmlGui as well... Overall it looks more and more like welcome consistency to me.</pre>
</blockquote>
<p>On November 29th, 2013, 9:01 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;">Ok, so to reiterate:
lib names -> libKF5Archive, libKF5Config etc (capital F)
target names -> KF5::Archive, KF5::Config
package names -> KF5Archive, KF5Config
correct?</pre>
</blockquote>
<p>On November 29th, 2013, 9:05 a.m. UTC, <b>Kevin Ottens</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;">Works for me if no one has any problem with it.</pre>
</blockquote>
<p>On November 29th, 2013, 9:18 a.m. UTC, <b>Kevin Ottens</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;">Oh, BTW, once we go for it, don't forget to cover plasma-framework too! Probably requires poking on #plasma or plasma-devel, it's kind of an "all or nothing" for the whole KDE Frameworks product.</pre>
</blockquote>
<p>On November 29th, 2013, 9:50 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;">Looks nice for the examples you gave, but let's take KIO:
KIOCore will become lib=libKF5IOCore target=KF5::IOCore in package=KF5IO? This dilutes the well-known name "KIO"...
Maybe in this particular case we want to repeat the K, i.e. libKF5KIOCore, KF5::KIOCore.
libKF5Parts also won't look like KParts anymore. Well, I guess that one is a matter of getting used to :)</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 see the point of those two where it's actually part of the brand name... That sais, one could argue it's the same for KConfig or KArchive really... So where is the line?</pre>
<br />
<p>- Kevin</p>
<br />
<p>On November 28th, 2013, 1:46 p.m. UTC, Martin Klapetek 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 Frameworks.</div>
<div>By Martin Klapetek.</div>
<p style="color: grey;"><i>Updated Nov. 28, 2013, 1:46 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</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 just one framework - KConfig - if this is the proper way to do this, I'll update this review with all frameworks updated to this.</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.</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>tier1/kconfig/autotests/CMakeLists.txt <span style="color: grey">(cc2626b)</span></li>
<li>tier1/kconfig/src/core/CMakeLists.txt <span style="color: grey">(c8a4842)</span></li>
<li>tier1/kconfig/src/gui/CMakeLists.txt <span style="color: grey">(b677d03)</span></li>
<li>tier1/kconfig/src/kconf_update/CMakeLists.txt <span style="color: grey">(69668bc)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/114182/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>