----------------------------------------------------------- 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 > >
_______________________________________________ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
