<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/103183/">http://git.reviewboard.kde.org/r/103183/</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 22nd, 2011, 12:22 a.m., <b>George Kiagiadakis</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, let's see. I'm ok for making it a library. That would happen anyway at some point...
What I see problematic here is the names. The whole naming scheme we use is a bit confusing.
The project is officially now "KDE-Telepathy".
Apps & tarballs (and l10n modules) are named "telepathy-kde-*".
The namespace used here and in other places is "KTelepathy" (which it's a bit long imho... how about KTp?).
This library is called libtelepathykdecommoninternalsprivate (record for biggest library name ever? :P), its cmake variables and export macros in this patch are KTELEPATHY_* and installs stuff in $prefix/include/telepathy-1.0/KTelepathy
The text-ui has a library called "ktelepathy_chat_lib", with export macros "KDE_TELEPATHY_CHAT_LIB_*" and installs headers in $prefix/include/KDETelepathy.
The nepomuk library that the other George is writing is called "ktelepathy" and installs headers in $prefix/include/KTelepathy (and has no proper export macro).
Now, let's make a policy out of this...
Btw, why the models have no namespace?</pre>
</blockquote>
<p>On November 22nd, 2011, 3:53 p.m., <b>Dario Freddi</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;">Whereas the naming concern is valid, consider the library at the moment is meant to remain private, so this is not an issue at the moment, and the name is intentionally confusing exactly to scare other people away from using it. When and if it will go public, of course we need to revise that part.
KTp namespace sounds ok with me, will discuss on IRC now.
The models have no namespace as changing namespace requires some changes to the Metatype handling, which historically doesn't play along well with them. However, that library is also going to change, as we want to export the models only and not the internal impl (items and such). When this happens, putting the exported classes in a namespace and the logic outside should be much easier.</pre>
</blockquote>
<p>On November 22nd, 2011, 5:22 p.m., <b>George Kiagiadakis</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 then. I'm still concerned about the include dir, though. Why use tp-qt4's include dir?</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;">That's simply convenience. Since that variable always points to the base telepathy dir includes (include/telepathy-1.0 for example), I thought it was a good idea to use that directly. I can change to {include}/telepathy-1.0 if you like</pre>
<br />
<p>- Dario</p>
<br />
<p>On November 21st, 2011, 7:54 p.m., Dario Freddi wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy and George Goldberg.</div>
<div>By Dario Freddi.</div>
<p style="color: grey;"><i>Updated Nov. 21, 2011, 7:54 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;">This patch turns common-internals into a library. Here follows a shortlog of changes of what I did for making this happen:
264472c Add CMake magic for making the library build
bb7e95b Now that everything has been put into a namespace, rename KPresence to Presence for consistency
ad52ffe Export the classes
b587540 Add KTelepathy namespace to base classes
b3ee5f6 Fix all warnings
83b2a68 Enforce the use of QLatin1*
5c8fc85 Enforce the non-use of Qt keywords
Missing:
* Fix paths (so move all the source code in a KTelepathy/ subfolder) for fixing includes in header files when including from other applications. I didn't do that in the review since it would have screwed up the diff badly.
* Port applications
George, I know you are reluctant towards this change, but the situation with the current submodules is getting unsustainable, and since we aim towards moving KTelepathy to Extragear for 0.3, we need to fix that. The concern about BC is not a problem, since we'll handle the SONUMBER accordingly, and the library will still be private for internal use until we decide for something different.</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">(PRE-CREATION)</span></li>
<li>circular-countdown.h <span style="color: grey">(039ce75)</span></li>
<li>circular-countdown.cpp <span style="color: grey">(0821e4c)</span></li>
<li>cmake/modules/COPYING-CMAKE-SCRIPTS <span style="color: grey">(PRE-CREATION)</span></li>
<li>cmake/modules/FindTelepathyQt4.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>error-dictionary.h <span style="color: grey">(3444539)</span></li>
<li>error-dictionary.cpp <span style="color: grey">(e3893e5)</span></li>
<li>global-presence.h <span style="color: grey">(d782c3e)</span></li>
<li>global-presence.cpp <span style="color: grey">(61f1457)</span></li>
<li>kpresence.h <span style="color: grey">(81e702f)</span></li>
<li>kpresence.cpp <span style="color: grey">(609ac24)</span></li>
<li>ktelepathy-export.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>models/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>models/accounts-filter-model.h <span style="color: grey">(8b55abb)</span></li>
<li>models/accounts-filter-model.cpp <span style="color: grey">(0665800)</span></li>
<li>models/accounts-model-item.h <span style="color: grey">(e3748b7)</span></li>
<li>models/accounts-model-item.cpp <span style="color: grey">(d854d8e)</span></li>
<li>models/accounts-model.h <span style="color: grey">(825f113)</span></li>
<li>models/accounts-model.cpp <span style="color: grey">(21f14cb)</span></li>
<li>models/contact-model-item.h <span style="color: grey">(c69d12f)</span></li>
<li>models/contact-model-item.cpp <span style="color: grey">(67de9fa)</span></li>
<li>models/groups-model-item.cpp <span style="color: grey">(cbe3da2)</span></li>
<li>models/groups-model.h <span style="color: grey">(ef9b28e)</span></li>
<li>models/groups-model.cpp <span style="color: grey">(e56dc2a)</span></li>
<li>models/proxy-tree-node.h <span style="color: grey">(ae123d1)</span></li>
<li>models/proxy-tree-node.cpp <span style="color: grey">(e413509)</span></li>
<li>models/tree-node.h <span style="color: grey">(9f675ab)</span></li>
<li>models/tree-node.cpp <span style="color: grey">(f892d5a)</span></li>
<li>presence.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>presence.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>service-availability-checker.h <span style="color: grey">(8883fe6)</span></li>
<li>service-availability-checker.cpp <span style="color: grey">(9b9e1d5)</span></li>
<li>telepathy-handler-application.h <span style="color: grey">(455463d)</span></li>
<li>telepathy-handler-application.cpp <span style="color: grey">(ce28c26)</span></li>
<li>text-parser.h <span style="color: grey">(7790b14)</span></li>
<li>text-parser.cpp <span style="color: grey">(510c2a5)</span></li>
<li>wallet-interface.h <span style="color: grey">(629dbe1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103183/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>