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

_______________________________________________
Okular-devel mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/okular-devel

Reply via email to