[Okular-devel] Review Request: Remember position on the page in bookmark

[Albert Astals Cid]

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


Looks good for a first attempt :-) Some comments inline


core/bookmarkmanager.h
<http://git.reviewboard.kde.org/r/104365/#comment9396>

    There is an @since marker missing here



core/bookmarkmanager.h
<http://git.reviewboard.kde.org/r/104365/#comment9397>

    @since missing



core/bookmarkmanager.h
<http://git.reviewboard.kde.org/r/104365/#comment9398>

    @since missing



core/bookmarkmanager.h
<http://git.reviewboard.kde.org/r/104365/#comment9399>

    @since missing



core/bookmarkmanager.h
<http://git.reviewboard.kde.org/r/104365/#comment9400>

    @since missing



core/bookmarkmanager.h
<http://git.reviewboard.kde.org/r/104365/#comment9401>

    This function makes no sense in the bookmarkmanager. Please move it to the only place you use it (part.cpp) and put it there as static



core/bookmarkmanager.cpp
<http://git.reviewboard.kde.org/r/104365/#comment9402>

    mark as static



core/bookmarkmanager.cpp
<http://git.reviewboard.kde.org/r/104365/#comment9403>

    This function doesn't seem to be used anymore, kill it



core/bookmarkmanager.cpp
<http://git.reviewboard.kde.org/r/104365/#comment9404>

    This is an unneeded an undocumented behaviour change, please move the sorting to the places where bookmarks() is called and having a defined order matters (i.e. the new calls in part.cpp you added)



part.cpp
<http://git.reviewboard.kde.org/r/104365/#comment9395>

    Mark this as const


- Albert Astals Cid


On March 22, 2012, 3:14 p.m., Mailson Menezes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104365/
> -----------------------------------------------------------
> 
> (Updated March 22, 2012, 3:14 p.m.)
> 
> 
> Review request for Okular and Albert Astals Cid.
> 
> 
> Description
> -------
> 
> This feature will remember the position where the user saved a bookmark. Also, will allow the user have multiple bookmarks per page.
> 
> 
> This addresses bug 157198.
>     http://bugs.kde.org/show_bug.cgi?id=157198
> 
> 
> Diffs
> -----
> 
>   core/bookmarkmanager.h 21bf34665b6c28877c13e6e8b759e10f76af5305 
>   core/bookmarkmanager.cpp 2d9c9c0134720689764c7dd858ea600de757eb3d 
>   part.h 1aafe265b7afa5a138aae4c9c0d41305456efff8 
>   part.cpp aee1d42ab2c8d30acaec2cd33821523402003a31 
> 
> Diff: http://git.reviewboard.kde.org/r/104365/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mailson Menezes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20120325/0be6e5db/attachment-0001.html>