Review request for kio_bookmarks

[Olivier Goffart]

Le dimanche 31 août 2008, Xavier Vello a écrit :
> Hello
>
> I think it's time to review kio_bookmarks for kdebase-runtime integration.
> You can read about it at http://commit-digest.org/issues/2008-08-10/
> I already did releases at
> http://kde-apps.org/content/show.php?content=86516 There are still a few
> quirks but it's already quite usable.
>
> A little checklist :
>   - compiles and runs with 4.1 and trunk
>   - user documentaion for the kio and the kcm in kio_bookmarks/doc (not
> installable by itself, but to be copied to kdebase/runtime/doc)
>   - one krazy issue, but it seems to be a false positive (missing
> "computer" icon while kickoff uses it)
>   - completely translatable
>
> This is my first Qt/KDE software, please tell me if I have coded things
> wrong. I'm subscribed to the list and on irc (xvello) if you need to
> contact me.

Hi, 
It looks pretty good.  I think it can be moved to kdebase.

Few comments anyway:

- Why are all your method virtual?  
It's bad practice because virtual function make the virtual table bigger, and 
add a redirection when calling it, so you should try to avoid using virtual 
if possible. (even if i agree that in this case, those performance issues are 
almost negligible)

- You could add yourself as author in the KAboutData

- it seems to me that echoHead("bookmarks:/") generate unfinished html


-- 
Olivier
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080903/b4f75e18/attachment.sig>