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

[Albert Astals Cid]

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


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?


core/document.cpp
<http://git.reviewboard.kde.org/r/107047/#comment20446>

    No yoda syntax for ifs please :-)



ui/presentationwidget.cpp
<http://git.reviewboard.kde.org/r/107047/#comment20445>

    Does the shorted "observerId()" work here?


- Albert Astals Cid


On Feb. 10, 2013, 9:51 p.m., Bogdan Cristea wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107047/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2013, 9:51 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/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 
>   ui/thumbnaillist.h 20c434f 
>   ui/thumbnaillist.cpp 33a5431 
>   ui/toc.h 8679648 
>   ui/toc.cpp bde7c97 
>   CMakeLists.txt e40cfd6 
>   active/components/pageitem.cpp a04a8dc 
>   core/document.h 1d825e1 
>   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 
>   generators/chm/generator_chm.cpp c342a10 
>   part.h e3a9418 
>   ui/annotationmodel.cpp d6d234d 
>   ui/bookmarklist.h cb8fcc3 
>   ui/bookmarklist.cpp e1b3869 
> 
> 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/20130210/fc9cbce3/attachment.html>