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