[Okular-devel] Review Request 107047: Settings separation: observer.h install and compilation correction on Windows

[Bogdan Cristea]

> On Feb. 10, 2013, 10:49 p.m., Albert Astals Cid wrote:
> > 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?

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).


- Bogdan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107047/#review27146
-----------------------------------------------------------


On Feb. 11, 2013, 9:07 p.m., Bogdan Cristea wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107047/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2013, 9:07 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Description
> -------
> 
> 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
> 
> 
> Diffs
> -----
> 
>   ui/thumbnaillist.h 20c434f 
>   ui/thumbnaillist.cpp 33a5431 
>   ui/toc.h 8679648 
>   ui/toc.cpp bde7c97 
>   generators/chm/generator_chm.cpp c342a10 
>   part.h e3a9418 
>   ui/annotationmodel.cpp d6d234d 
>   ui/bookmarklist.h cb8fcc3 
>   ui/bookmarklist.cpp e1b3869 
>   ui/minibar.h a0c0514 
>   ui/pagepainter.cpp 91ae211 
>   ui/pagesizelabel.h 7c4a1e2 
>   ui/pageview.h d8a7653 
>   ui/pageview.cpp 60a273d 
>   ui/presentationwidget.h 1608ef8 
>   ui/presentationwidget.cpp 35b9d34 
>   ui/side_reviews.h bbd8324 
>   core/document.cpp 372af56 
>   core/generator.cpp 402c881 
>   core/observer.h f7189be 
>   core/observer.cpp 59bbb11 
>   core/page.h 6bc60c5 
>   core/page.cpp 4df58e0 
>   core/page_p.h 75575a7 
>   CMakeLists.txt e40cfd6 
>   active/components/pageitem.cpp a04a8dc 
>   core/document.h 1d825e1 
> 
> Diff: http://git.reviewboard.kde.org/r/107047/diff/
> 
> 
> Testing
> -------
> 
> no
> 
> 
> Thanks,
> 
> Bogdan Cristea
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20130214/db39f777/attachment.html>