Moving Baloo and Baloo-widgets into KDE SC
Vishesh Handa
me at vhanda.in
Mon Jan 20 17:43:13 GMT 2014
On Sunday 19 January 2014 15:56:17 David Edmundson wrote:
> Code Review of baloo/file/lib
>
> ==file.cpp
>
> Should it override type() from Item and set it to "File" ?
>
I'll probably removing this whole type thing. I'm not sure.
>
> ==filemodifyjob.cpp.
>
> The code won't unset a rating, comments or tags on multiple files.
> You update Xapian ok, but you're not calling fsetxattr().
>
Right. I think I'll need to re-architecture the internals. There needs to be a
way to differentiate between removing the rating and the user not setting
them.
Eg - File f(someUrl); f.setRating(5); save f;
In this case the comment and tags would be removed even though they should
not.
> The d pointer leaks?
>
Fixed d-pointer.
> ==DB.cpp
> SQLITE3-> SQLITE
>
Fixed.
> ==general
>
> Are you planning on putting all the Xapian queries in a new thread in
> the future? If not having a KJob API doesn't have any benefit does it?
>
Not right now. Xapian blocks for a very very small amount. In the future if
required it can be moved to another thread.
> I would suggest you namespace your header guards
> i.e
> #ifndef FILE_H -> #ifndef BALOO_FILE_H
> The current ones are very generic, it will be very easy to accidentally
> clash.
>
Thanks. Fixed.
> David
>
>
> David
--
Vishesh Handa
More information about the kde-core-devel
mailing list