<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/107047/">http://git.reviewboard.kde.org/r/107047/</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 10th, 2013, 10:49 p.m. UTC, <b>Albert Astals Cid</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;">The patch looks ok minor the two small inline comments.
Having a look at the patch though i see we have those *special* ids, and i think it would make much more sense not to have any, that would mean having a look at what those special ids do and splitting it into separate options of either observer, or the function calls or whatever. But maybe this is too much for this and we should just commit this first and then try to generalize those?
What do you all think?</pre>
</blockquote>
<p>On February 14th, 2013, 6:23 a.m. UTC, <b>Bogdan Cristea</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;">You need to provide anyway the maximum number of public IDs for Document class in order to generate private IDs outside the reserved range. So I believe that is better to have public IDs and their number in one place, i.e. in an enumeration inside Document class. For the moment I would just commit these changes as they are and open maybe a feature request or whatever in order to remove completely these IDs (I do not see yet how it might be done).</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'll commit the changes, but let's think about why do we need private ids at all?</pre>
<br />
<p>- Albert</p>
<br />
<p>On February 11th, 2013, 9:07 p.m. UTC, Bogdan Cristea 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 Okular.</div>
<div>By Bogdan Cristea.</div>
<p style="color: grey;"><i>Updated Feb. 11, 2013, 9:07 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 is related to settings separation for the frontend and the backend. It proposes the installation of core/observer.h and corrects compilation of okular on Windows (using KDE on windows):
- after separation, settings.h and settings_core.h need to use different precompiler switches for exporting/importing symbols
- add definitions needed to activate these switches on Windows
</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;">no</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>ui/thumbnaillist.h <span style="color: grey">(20c434f)</span></li>
<li>ui/thumbnaillist.cpp <span style="color: grey">(33a5431)</span></li>
<li>ui/toc.h <span style="color: grey">(8679648)</span></li>
<li>ui/toc.cpp <span style="color: grey">(bde7c97)</span></li>
<li>generators/chm/generator_chm.cpp <span style="color: grey">(c342a10)</span></li>
<li>part.h <span style="color: grey">(e3a9418)</span></li>
<li>ui/annotationmodel.cpp <span style="color: grey">(d6d234d)</span></li>
<li>ui/bookmarklist.h <span style="color: grey">(cb8fcc3)</span></li>
<li>ui/bookmarklist.cpp <span style="color: grey">(e1b3869)</span></li>
<li>ui/minibar.h <span style="color: grey">(a0c0514)</span></li>
<li>ui/pagepainter.cpp <span style="color: grey">(91ae211)</span></li>
<li>ui/pagesizelabel.h <span style="color: grey">(7c4a1e2)</span></li>
<li>ui/pageview.h <span style="color: grey">(d8a7653)</span></li>
<li>ui/pageview.cpp <span style="color: grey">(60a273d)</span></li>
<li>ui/presentationwidget.h <span style="color: grey">(1608ef8)</span></li>
<li>ui/presentationwidget.cpp <span style="color: grey">(35b9d34)</span></li>
<li>ui/side_reviews.h <span style="color: grey">(bbd8324)</span></li>
<li>core/document.cpp <span style="color: grey">(372af56)</span></li>
<li>core/generator.cpp <span style="color: grey">(402c881)</span></li>
<li>core/observer.h <span style="color: grey">(f7189be)</span></li>
<li>core/observer.cpp <span style="color: grey">(59bbb11)</span></li>
<li>core/page.h <span style="color: grey">(6bc60c5)</span></li>
<li>core/page.cpp <span style="color: grey">(4df58e0)</span></li>
<li>core/page_p.h <span style="color: grey">(75575a7)</span></li>
<li>CMakeLists.txt <span style="color: grey">(e40cfd6)</span></li>
<li>active/components/pageitem.cpp <span style="color: grey">(a04a8dc)</span></li>
<li>core/document.h <span style="color: grey">(1d825e1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107047/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>